Skip to content

Feature/refactor code smells - #97

Open
Rokib1605 wants to merge 2 commits into
jhalterman:masterfrom
Rokib1605:feature/refactor-code-smells
Open

Feature/refactor code smells#97
Rokib1605 wants to merge 2 commits into
jhalterman:masterfrom
Rokib1605:feature/refactor-code-smells

Conversation

@Rokib1605

Copy link
Copy Markdown

Summary
This pull request applies five structural refactoring improvements to ExpiringMap.java to improve readability, reduce complexity, and better satisfy SOLID design principles. All 35 existing tests pass unchanged, and no public API signatures were modified.
Problems Identified
God Class: ExpiringMap.java at 1,427 lines handles six unrelated responsibilities simultaneously, violating SRP
Long Methods: put() and related methods exceed 40 lines mixing multiple operations
Deep Nesting: get(), scheduleEntry() contain 3-4 levels of nested conditionals
Duplicate Lock Pattern: Identical try/finally lock boilerplate repeated across 15+ methods, violating DRY
Inner Class Coupling: ExpiringEntry defined inside ExpiringMap prevents independent testing and reuse
Ambiguous Names: expirationNanos, listenerService, expirer do not clearly communicate intent
Refactoring Techniques Applied
Extract Method: put() decomposed into putInternal(), createAndScheduleEntry(), updateExistingEntry()
Guard Clauses: Nested conditionals in get() replaced with early returns
Rename Variable: Five ambiguous identifiers renamed to clearly express intent
Extract Helper: Duplicate lock pattern consolidated into withWriteLock() functional helper
Extract Class: ExpiringEntry moved to its own package-private file
SOLID Principles Improved
SRP: Each extracted method now has one clear responsibility
OCP: withWriteLock() helper allows locking strategy to change without touching every method
DRY: Lock boilerplate defined once instead of 15 times
Testing Evidence
Before refactoring: Tests run: 35, Failures: 0, Errors: 0 — BUILD SUCCESS
After refactoring: Tests run: 35, Failures: 0, Errors: 0 — BUILD SUCCESS
All public API signatures remain identical. No test files were modified.
Impact on Maintainability
Future contributors can now understand put() in under 10 seconds instead of tracing 40 lines. The extracted ExpiringEntry class can be tested and documented independently. The withWriteLock() helper means any future locking strategy change requires editing one method instead of fifteen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant