Refactoring Java Login Security: A Real-World E-Commerce Case Study

Business Description

The existing system already provides a login feature; the new requirement is to add security constraints that limit login by IP (internal or external) and by allowed login time.

Original Code Issues Analysis

The original implementation mixes several concerns and contains many problems:

String‑based matching is used where a simple boolean check would suffice.

Time handling (hh:MM) is duplicated and should be extracted to a utility class.

Repeated comma‑separated string splitting appears in many places and should be centralized.

Three business rules (time window, internal IP, external IP) are tangled together in nested loops, making the code hard to read and modify.

SecureLogEvent objects are instantiated without passing key business data.

Comments are scarce, reducing readability of critical sections.

Method names are vague and do not convey intent.

Magic numbers/strings are scattered instead of being defined as constants.

Refactoring Process Explanation

The refactoring was performed step by step:

Read the original code and identify duplicated logic.

Extract time‑range validation into a private helper method.

Write a small main test to verify the new method before replacing the old code.

Identify common string‑splitting logic, create a private utility method, test it, and replace the duplicated code.

Separate the “allowed login time” check from the IP checks; it should be evaluated first and independently.

Split internal‑IP and external‑IP checks into two independent loops.

After clarifying the business rules, the code hierarchy becomes clearer.

Enhance logging by adding meaningful business data, add comments, and replace magic literals with constants.

Change the call site (Figure 1) to use a boolean‑returning method.

Move the whole rule‑checking logic out of CreditController into a dedicated utility class, reducing controller size and improving future extensibility.

Refactored Code Overview

The refactored version introduces a utility class with clearly named constants and a single public method. The method first checks the allowed time range via forbidVisitTimeRange, then calls isInside and isOutside which each delegate to a private checkRange method, eliminating the previous double loops. Logging is handled by a private recordLog method that accepts a concatenated business‑data string.

Conclusion

Avoid duplicated code by extracting reusable utilities.

Leverage existing framework or third‑party utilities instead of reinventing wheels.

When multiple loops are present, consider whether each business rule can be isolated into its own method.

Provide comments for critical sections to improve readability.

Replace magic literals with well‑named constants.

Write logs that contain sufficient context to reproduce issues later.