Refactored: Implementation and Design Smells #1636

akshat799 · 2025-03-28T16:28:20Z

Implementation Smell Refactoring:

Refactoring Type: Extract Method

Location: initializr-web/src/main/java/io/spring/initializr/web/support/DefaultDependencyMetadataProvider.java

```
Smell Type: Complex Method
```

Class Name: DefaultDependencyMetadataProvider

```
Method Name: get
```
```
Line: 41
```

Reason: The original `DefaultDependencyMetadataProvider` class had a `get` method with a cyclomatic complexity of 9, making it complex, hard to read, and difficult to test. Refactoring using the Extract Method technique split it into `getDependencies`, `getRepositoriesDependencies`, and `getBoms`, reducing complexity and improving maintainability. This enhanced readability, reusability, and testability while adhering to the Single Responsibility Principle.

Refactoring Type: Decompose conditional

Location: initializr-generator/src/main/java/io/spring/initializr/generator/buildsystem/maven/MavenBuildWriter.java

```
Smell Type: Complex Conditional
```
```
Class Name: MavenBuildWriter
```
```
Method Name: get
```
```
Line: 358
```

Reason: The original code had a long and complex conditional statement checking multiple attributes of `MavenBuild` and `MavenBuildSettings`, making it harder to read and maintain. To improve clarity, I extracted separate methods—`isBuildEmpty()` and `isBuildSettingsEmpty()`—to handle the different aspects of the condition. This decomposition makes the logic more readable, modular, and easier to modify in the future. Now, the `writeBuild` method focuses on its primary purpose while delegating condition checks to dedicated helper methods.

Refactoring Type: Introduce explaining variable

Location: initializr-generator/src/main/java/io/spring/initializr/generator/version/VersionParser.java

```
Smell Type: Complex Conditional
```
```
Class Name: VersionParser
```
```
Method Name: findLatestVersion
```
```
Line: 143
```

Reason: Introducing explaining variables breaks down complex logic into smaller, self-explanatory parts, which improves the overall readability of the code. By refactoring the lambda in the findLatestVersion method to as#termediate boolean values, it becomes clear what each condition is checking for, reducing cognitive load for future maintainers. This method of refactoring not only simplifies understanding but also aids in debugging and modifying the logic as requirements evolve.

Design Smell Refactoring:

Refactoring Type: Pull Up Method

Location: initializr-generator/src/main/java/io/spring/initializr/generator/buildsystem/gradle/GradleBuild.java , initializr-generator/src/main/java/io/spring/initializr/generator/buildsystem/maven/MavenBuild.java

```
Class Name: GradleBuild and MavenBuild
```

Smell Type: Duplicate Code / Missing Abstraction

```
Method Name: plugins, extensions
```

New Interface Name: BuildExtensionsAndPlugins

New Interface Location: initializr-generator/src/main/java/io/spring/initializr/generator/buildsystem/BuildExtensionsAndPlugins.java

Reason: The purpose of creating this interface is to enforce a common contract between the build implementations. By abstracting out the common methods (extensions() and plugins()), we reduce duplicated code and simplify client code that interacts with both Maven and Gradle builds, allowing for easier maintenance and extension in the future.

Refactoring Type: Extract Class

Location: initializr-web/src/main/java/io/spring/initializr/web/project/DefaultProjectRequestToDescriptionConverter.java

Class Name: DefaultProjectRequestToDescriptionConverter

Smell Type: Long Class/ Feature Concentration

Method Name: validate, validatePlatformVersion, validateType, validateLanguage, validatePackaging, validateDependencies

New Class Name: ProjectRequestValidator

New Class Location: initializr-web/src/main/java/io/spring/initializr/web/project/ProjectRequestValidator.java

Reason: I refactored the code by extracting the validation logic from the DefaultProjectRequestToDescriptionConverter class into a new ProjectRequestValidator class to separate responsibilities, improving maintainability, readability, and testability.

Refactoring Type: Push Down Method

Location: initializr-generator/src/main/java/io/spring/initializr/generator/language/Language.java

```
Interface Name: Language
```
```
Smell Type: Unutilized Abstraction
```
```
Method Name:jvmVersion
```

Class Name where the methods are shifted to: AbstractLanguage

Class Location: initializr-generator/src/main/java/io/spring/initializr/generator/language/AbstractLanguage.java

Reason: The language interface was implemented by 4 different classes but utilized by only abstractLanguage by Overriding it. So I am moving the jvmversion method to this abstract class.

…t method into smaller methods following SRP using Extract Method

…r the get method into smaller methods following SRP using Extract Method" This reverts commit 42ce2da.

…t method into smaller methods following SRP using Extract Method

…onal checks in 2 methods

…tVersion lambda

…p method refactoring for Gradle and Maven Build

…tractClass

mhalbritter · 2025-03-31T07:06:35Z

There are a lot of new POM files (e.g. initializr-actuator/.flattened-pom 2.xml), please remove those.

mhalbritter

Hello @akshat799 , I've left some comments for your consideration.

mhalbritter · 2025-03-31T07:09:02Z

...io/spring/initializr/generator/spring/build/gradle/GradleProjectGenerationConfiguration.java

@@ -86,7 +87,8 @@ private GradleBuild createGradleBuild(BuildItemResolver buildItemResolver,

 	@Bean
 	public BuildCustomizer<GradleBuild> defaultGradleBuildCustomizer(ProjectDescription description) {
-		return (build) -> build.settings().sourceCompatibility(description.getLanguage().jvmVersion());
+		return (build) -> build.settings()


I don't see how that improves things. Please revert.

mhalbritter · 2025-03-31T07:09:06Z

...main/java/io/spring/initializr/generator/spring/build/maven/DefaultMavenBuildCustomizer.java

@@ -46,7 +47,7 @@ public DefaultMavenBuildCustomizer(ProjectDescription description, InitializrMet
 	public void customize(MavenBuild build) {
 		build.settings().addOverrideIfEmpty(true);
 		build.settings().name(this.description.getName()).description(this.description.getDescription());
-		build.properties().property("java.version", this.description.getLanguage().jvmVersion());
+		build.properties().property("java.version", ((AbstractLanguage) this.description.getLanguage()).jvmVersion());


I don't see how that improves things. Please revert.

mhalbritter · 2025-03-31T07:09:12Z

.../io/spring/initializr/generator/spring/code/kotlin/KotlinProjectGenerationConfiguration.java

@@ -100,7 +100,8 @@ public KotlinProjectSettings kotlinProjectSettings(ObjectProvider<KotlinVersionR
 		String kotlinVersion = kotlinVersionResolver
 			.getIfAvailable(() -> new InitializrMetadataKotlinVersionResolver(metadata))
 			.resolveKotlinVersion(this.description);
-		return new SimpleKotlinProjectSettings(kotlinVersion, this.description.getLanguage().jvmVersion());
+		return new SimpleKotlinProjectSettings(kotlinVersion,


I don't see how that improves things. Please revert.

mhalbritter · 2025-03-31T07:09:39Z

...ializr-generator/src/main/java/io/spring/initializr/generator/language/AbstractLanguage.java

@@ -46,7 +46,7 @@ public String id() {
 		return this.id;
 	}

-	@Override
+	// @Override


Please revert.

mhalbritter · 2025-03-31T07:09:42Z

initializr-generator/src/main/java/io/spring/initializr/generator/language/Language.java

-	 * @return the JVM version or {@value DEFAULT_JVM_VERSION} if not set
-	 */
-	String jvmVersion();
+	// /**


Please revert.

mhalbritter · 2025-03-31T07:14:18Z

...ator/src/main/java/io/spring/initializr/generator/buildsystem/BuildExtensionsAndPlugins.java

+ * @param <P> the type of the plugin container
+ * @author Akshat Gulati
+ */
+public interface BuildExtensionsAndPlugins<E, P> {


What's the reason for that abstraction? I can't see any usages of it.

mhalbritter · 2025-03-31T07:17:16Z

initializr-generator/src/test/java/io/spring/initializr/generator/language/LanguageTests.java

@@ -37,7 +37,7 @@ void javaLanguage() {
 		assertThat(java).isInstanceOf(JavaLanguage.class);
 		assertThat(java.id()).isEqualTo("java");
 		assertThat(java.toString()).isEqualTo("java");
-		assertThat(java.jvmVersion()).isEqualTo("11");
+		assertThat(((JavaLanguage) java).jvmVersion()).isEqualTo("11");


I don't see how that improves things. Please revert.

mhalbritter · 2025-03-31T07:17:19Z

initializr-generator/src/test/java/io/spring/initializr/generator/language/LanguageTests.java

@@ -46,7 +46,7 @@ void kotlinLanguage() {
 		assertThat(kotlin).isInstanceOf(KotlinLanguage.class);
 		assertThat(kotlin.id()).isEqualTo("kotlin");
 		assertThat(kotlin.toString()).isEqualTo("kotlin");
-		assertThat(kotlin.jvmVersion()).isEqualTo("1.8");
+		assertThat(((KotlinLanguage) kotlin).jvmVersion()).isEqualTo("1.8");


I don't see how that improves things. Please revert.

mhalbritter · 2025-03-31T07:17:22Z

initializr-generator/src/test/java/io/spring/initializr/generator/language/LanguageTests.java

@@ -55,7 +55,7 @@ void groovyLanguage() {
 		assertThat(groovy).isInstanceOf(GroovyLanguage.class);
 		assertThat(groovy.id()).isEqualTo("groovy");
 		assertThat(groovy.toString()).isEqualTo("groovy");
-		assertThat(groovy.jvmVersion()).isEqualTo("1.8");
+		assertThat(((GroovyLanguage) groovy).jvmVersion()).isEqualTo("1.8");


I don't see how that improves things. Please revert.

mhalbritter · 2025-03-31T07:18:25Z

.../java/io/spring/initializr/web/project/DefaultProjectRequestToDescriptionConverterTests.java

@@ -274,7 +275,7 @@ void convertShouldSetLanguageForProjectDescriptionFromRequest() {
 		request.setJavaVersion("1.8");
 		ProjectDescription description = this.converter.convert(request, this.metadata);
 		assertThat(description.getLanguage().id()).isEqualTo("java");
-		assertThat(description.getLanguage().jvmVersion()).isEqualTo("1.8");
+		assertThat(((AbstractLanguage) description.getLanguage()).jvmVersion()).isEqualTo("1.8");


I don't see how that improves things. Please revert.

mhalbritter · 2025-03-31T07:19:28Z

You also need to sign the commits, as explained here.

akshat799 added 8 commits March 27, 2025 20:30

Refactoring(DefaultDependencyMetadataProvider - get): Refactor the ge…

42ce2da

…t method into smaller methods following SRP using Extract Method

Revert "Refactoring(DefaultDependencyMetadataProvider - get): Refacto…

4642ac0

…r the get method into smaller methods following SRP using Extract Method" This reverts commit 42ce2da.

Refactoring(DefaultDependencyMetadataProvider - get): Refactor the ge…

19b1231

…t method into smaller methods following SRP using Extract Method

Refactoring(MavenBuildWriter - writeBuild): decompose complex conditi…

af5a293

…onal checks in 2 methods

refactor(version-parser): introduce explaining variables in findLates…

fdc69fe

…tVersion lambda

refactor(build): create BuildExtensionAndPlugins interface for pull u…

480c038

…p method refactoring for Gradle and Maven Build

refactor(project): extract validation logic to ProjectRequestValidator

5c38a81

Refactor(jvmVersion Language): push down getter for jvmVersion to abs…

f50280b

…tractClass

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 28, 2025

mhalbritter requested changes Mar 31, 2025

View reviewed changes

mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Mar 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored: Implementation and Design Smells #1636

Refactored: Implementation and Design Smells #1636

akshat799 commented Mar 28, 2025

mhalbritter commented Mar 31, 2025

mhalbritter left a comment

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter Mar 31, 2025

mhalbritter commented Mar 31, 2025 •

edited

Loading

Refactored: Implementation and Design Smells #1636

Are you sure you want to change the base?

Refactored: Implementation and Design Smells #1636

Conversation

akshat799 commented Mar 28, 2025

mhalbritter commented Mar 31, 2025

mhalbritter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhalbritter commented Mar 31, 2025 • edited Loading

mhalbritter commented Mar 31, 2025 •

edited

Loading