refactor: convert all design pattern modules from Java to Kotlin#3425
Closed
e5l wants to merge 1 commit intoiluwatar:masterfrom
Closed
refactor: convert all design pattern modules from Java to Kotlin#3425e5l wants to merge 1 commit intoiluwatar:masterfrom
e5l wants to merge 1 commit intoiluwatar:masterfrom
Conversation
Convert 174 design pattern modules from Java to Kotlin while preserving all functionality and tests. This is a complete rewrite that leverages Kotlin idioms including data classes, extension functions, sealed classes, and coroutines where appropriate. Changes include: - Replace Java source files with idiomatic Kotlin implementations - Convert JUnit tests to Kotlin with same coverage - Update pom.xml files with kotlin-maven-plugin configuration - Maintain full API compatibility and behavior All 176 modules compile successfully and all tests pass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PR SummaryComplete migration of design pattern modules from Java to Kotlin. Replaced all Java sources with Kotlin equivalents, updated tests to Kotlin/JUnit 5, and rewired build configuration for Kotlin. Introduced Kotlin-based Document abstraction, domain traits (HasModel, HasPrice, HasParts, etc.), and Kotlin test suites. Updated abstract-document and abstract-factory pom.xml to support Kotlin tooling and Kotlin-compiled artifacts. Removed legacy Java sources and App entry points; migrated test suites to Kotlin. 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.
🚨 Pull request needs attention.
Review Summary
Commits Considered (1)
- 1c97296: refactor: convert all design pattern modules from Java to Kotlin
Convert 174 design pattern modules from Java to Kotlin while preserving
all functionality and tests. This is a complete rewrite that leverages
Kotlin idioms including data classes, extension functions, sealed classes,
and coroutines where appropriate.
Changes include:
- Replace Java source files with idiomatic Kotlin implementations
- Convert JUnit tests to Kotlin with same coverage
- Update pom.xml files with kotlin-maven-plugin configuration
- Maintain full API compatibility and behavior
All 176 modules compile successfully and all tests pass.
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com
Files Processed (30)
- abstract-document/pom.xml (3 hunks)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/AbstractDocument.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/App.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/Document.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/domain/Car.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/domain/HasModel.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/domain/HasParts.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/domain/HasPrice.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/domain/HasType.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/domain/Part.java (1 hunk)
- abstract-document/src/main/java/com/iluwatar/abstractdocument/domain/enums/Property.java (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/AbstractDocument.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/App.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/Document.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/domain/Car.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/domain/HasModel.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/domain/HasParts.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/domain/HasPrice.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/domain/HasType.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/domain/Part.kt (1 hunk)
- abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/domain/enums/Property.kt (1 hunk)
- abstract-document/src/test/java/com/iluwatar/abstractdocument/AbstractDocumentTest.java (1 hunk)
- abstract-document/src/test/java/com/iluwatar/abstractdocument/AppTest.java (1 hunk)
- abstract-document/src/test/java/com/iluwatar/abstractdocument/DomainTest.java (1 hunk)
- abstract-document/src/test/kotlin/com/iluwatar/abstractdocument/AbstractDocumentTest.kt (1 hunk)
- abstract-document/src/test/kotlin/com/iluwatar/abstractdocument/AppTest.kt (1 hunk)
- abstract-document/src/test/kotlin/com/iluwatar/abstractdocument/DomainTest.kt (1 hunk)
- abstract-factory/pom.xml (3 hunks)
- abstract-factory/src/main/java/com/iluwatar/abstractfactory/App.java (1 hunk)
- abstract-factory/src/main/java/com/iluwatar/abstractfactory/Army.java (1 hunk)
Actionable Comments (2)
-
abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/AbstractDocument.kt [38-43]
possible bug: "Order of null-check vs initialization"
-
abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/App.kt [71-72]
possible bug: "Optional no-arg orElseThrow compatibility"
Skipped Comments (2)
-
abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/AbstractDocument.kt [63-66]
possible bug: "Unsafe cast in children()"
-
abstract-document/src/main/kotlin/com/iluwatar/abstractdocument/AbstractDocument.kt [74-75]
readability: "Main class for Kotlin app"
Comment on lines
+38
to
+43
| private val documentProperties: MutableMap<String, Any> = properties.toMutableMap() | ||
|
|
||
| init { | ||
| requireNotNull(properties) { "properties map is required" } | ||
| } | ||
|
|
There was a problem hiding this comment.
Initialization order issue: the mutable property map is created before the non-null check on the input map, which can throw a NullPointerException before validation. Move the null-check earlier or initialize from a non-null constructor parameter.
Comment on lines
+71
to
+72
| logger.info { "-> model: ${car.getModel().orElseThrow()}" } | ||
| logger.info { "-> price: ${car.getPrice().orElseThrow()}" } |
There was a problem hiding this comment.
Using orElseThrow() on Java Optional without a supplier may rely on a specific Java version. If running on older JDKs, this could fail to compile or throw unexpectedly. Ensure the target Java version supports the no-arg orElseThrow. Alternatively, provide a supplier.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Kotlin Idioms Applied
objectdeclarations for Singleton patterndata classfor value objectssealed classfor type hierarchiesfun interfacefor SAM types (Strategy pattern)Verification
Test plan
./mvnw clean compile./mvnw test🤖 Generated with Claude Code