refactor: apply Factory Enum Pattern for cleaner object instantiation#3523
refactor: apply Factory Enum Pattern for cleaner object instantiation#3523prashantpiyush1111 wants to merge 5 commits into
Conversation
PR SummaryCloses #3301. Applied the Factory Enum Pattern to the CoinType enum by introducing a per-type singleton via getInstance(); removed CoinFactory dependency from App.java; updated tests to instantiate coins via CoinType.getInstance() and added tests to verify singleton behavior. Changes
autogenerated by presubmit.ai |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- 7c3a3a0: refactor: apply Factory Enum Pattern for cleaner object instantiation
Files Processed (3)
- factory/src/main/java/com/iluwatar/factory/App.java (1 hunk)
- factory/src/main/java/com/iluwatar/factory/CoinType.java (2 hunks)
- factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java (1 hunk)
Actionable Comments (0)
Skipped Comments (3)
-
factory/src/main/java/com/iluwatar/factory/App.java [43-44]
possible bug: "Make singleton per coin type thread-safe"
-
factory/src/main/java/com/iluwatar/factory/CoinType.java [41-50]
possible bug: "Thread-safety of per-type singleton"
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [33-50]
testing: "Tests for CoinType#getInstance() behavior"
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- 106b065: style: fix formatting in CoinFactoryTest
Files Processed (1)
- factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java (1 hunk)
Actionable Comments (0)
Skipped Comments (3)
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [33-37]
readability: "Clarify singleton non-null contract in copper coin test."
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [39-43]
readability: "Improve parity and clarity for gold coin singleton test."
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [45-49]
readability: "Consolidate singleton consistency checks for both coin types."
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- 402abb4: refactor(factory): apply enum-based factory pattern
Files Processed (1)
- factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java (1 hunk)
Actionable Comments (0)
Skipped Comments (3)
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [33-37]
readability: "Tighten type assertion for Copper coin instance"
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [39-43]
readability: "Tighten type assertion for Gold coin instance"
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [45-49]
readability: "Confirm singleton semantics across both types"
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- 1e2350a: style(factory): fix test indentation
Files Processed (1)
- factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java (1 hunk)
Actionable Comments (0)
Skipped Comments (3)
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [33-37]
best_practice: "Singleton per-coin instance expectations"
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [39-43]
best_practice: "Singleton per-coin instance expectations"
-
factory/src/test/java/com/iluwatar/factory/CoinFactoryTest.java [45-49]
best_practice: "Ensure distinct singletons per coin type"
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Commits Considered (1)
- 569afc8: style(factory): add trailing newline
Files Processed (1)
- factory/src/main/java/com/iluwatar/factory/CoinType.java (2 hunks)
Actionable Comments (1)
-
factory/src/main/java/com/iluwatar/factory/CoinType.java [45-50]
possible bug: "Thread-safety concern in lazy initialization."
Skipped Comments (1)
-
factory/src/main/java/com/iluwatar/factory/CoinType.java [41-43]
readability: "Per-type singleton field lacks visibility guarantees; ensure proper memory semantics."
| public Coin getInstance() { | ||
| if (instance == null) { | ||
| instance = constructor.get(); | ||
| } | ||
| return instance; | ||
| } |
There was a problem hiding this comment.
getInstance() implements lazy initialization without synchronization, which is not thread-safe. In multi-threaded scenarios, two threads could observe instance == null and both create a Coin, potentially leaking one and returning a non-singleton. Consider a thread-safe lazy initialization strategy (e.g., double-checked locking with a volatile field, or initialize at enum construction).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3523 +/- ##
============================================
- Coverage 83.28% 83.28% -0.01%
- Complexity 4023 4024 +1
============================================
Files 1060 1060
Lines 14246 14249 +3
Branches 686 686
============================================
+ Hits 11865 11867 +2
- Misses 2094 2095 +1
Partials 287 287 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Closes #3301
Applied Factory Enum Pattern to CoinType enum: