Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: further refactoring for code health improvement #406

Merged
merged 13 commits into from
Nov 15, 2023

Conversation

CarstenWickner
Copy link
Member

@CarstenWickner CarstenWickner commented Nov 12, 2023

No description provided.

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 0 findings(s) 🚩
  • Improving Code Health: 6 findings(s) ✅
  • Affected Hotspots: 1 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 0 findings(s) 🚩
  • Improving Code Health: 6 findings(s) ✅
  • Affected Hotspots: 1 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: FAILED

  • Declining Code Health: 1 findings(s) 🚩
  • Improving Code Health: 12 findings(s) ✅
  • Affected Hotspots: 1 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

🚩 Declining Code Health (highest to lowest):

✅ Improving Code Health:

Comment on lines +349 to +358
private Map.Entry<ObjectNode, Boolean> applyReferencedCustomDefinition(CustomDefinition customDefinition, ResolvedType targetType,
ObjectNode targetNode, boolean isNullable, CustomDefinitionProviderV2 ignoredDefinitionProvider) {
ObjectNode definition = this.generatorConfig.createObjectNode();
this.putDefinition(targetType, definition, ignoredDefinitionProvider);
this.addReference(targetType, targetNode, ignoredDefinitionProvider, isNullable);
this.markDefinitionAsNeverInlinedIfRequired(customDefinition, targetType, ignoredDefinitionProvider);
logger.debug("applying configured custom definition for {}", targetType);
definition.setAll(customDefinition.getValue());
return new AbstractMap.SimpleEntry<>(definition, customDefinition.shouldIncludeAttributes());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ New issue: Excess Number of Function Arguments
applyReferencedCustomDefinition has 5 arguments, threshold = 4

Why does this problem occur?

This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments. Read more.

Comment on lines +360 to +362
private Map.Entry<ObjectNode, Boolean> applyStandardDefinition(boolean shouldInlineDefinition, TypeScope scope, ObjectNode targetNode,
boolean isNullable, CustomDefinitionProviderV2 ignoredDefinitionProvider) {
ResolvedType targetType = scope.getType();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ New issue: Excess Number of Function Arguments
applyStandardDefinition has 5 arguments, threshold = 4

Why does this problem occur?

This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments. Read more.

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 0 findings(s) 🚩
  • Improving Code Health: 12 findings(s) ✅
  • Affected Hotspots: 1 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: FAILED

  • Declining Code Health: 1 findings(s) 🚩
  • Improving Code Health: 32 findings(s) ✅
  • Affected Hotspots: 2 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

Absence of Expected Change Pattern

  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/MethodScope.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/FieldScope.java

🚩 Declining Code Health (highest to lowest):

✅ Improving Code Health:

@@ -19,12 +19,12 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.victools.jsonschema.generator.Module;
import com.github.victools.jsonschema.generator.Option;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Overall Code Complexity
The mean cyclomatic complexity in this module is no longer above the threshold

@@ -703,7 +706,7 @@ private JsonNode populateMethodSchema(MethodScope method) {
typeOverrides = this.generatorConfig.resolveSubtypes(method.getType(), this);
}
List<MethodScope> methodOptions;
if (typeOverrides == null || typeOverrides.isEmpty()) {
if (Util.isNullOrEmpty(typeOverrides)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Getting better: Complex Method
populateMethodSchema decreases in cyclomatic complexity from 10 to 9, threshold = 9

Why does this problem occur?

This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.

@@ -622,7 +625,7 @@ private ObjectNode populateFieldSchema(FieldScope field) {
typeOverrides = this.generatorConfig.resolveSubtypes(field.getType(), this);
}
List<FieldScope> fieldOptions;
if (typeOverrides == null || typeOverrides.isEmpty()) {
if (Util.isNullOrEmpty(typeOverrides)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Getting better: Complex Method
populateFieldSchema decreases in cyclomatic complexity from 10 to 9, threshold = 9

Why does this problem occur?

This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.

final CustomDefinition customDefinition = this.generatorConfig.getCustomDefinition(targetType, this, ignoredDefinitionProvider);
if (customDefinition != null && (customDefinition.isMeantToBeInline() || forceInlineDefinition)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Complex Conditional
traverseGenericType no longer has a complex conditional

Comment on lines -531 to -532
if ((!includeStaticFields || hierachyType.getStaticFields().isEmpty())
&& (!includeStaticMethods || hierachyType.getStaticMethods().isEmpty())) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Complex Conditional
collectObjectProperties no longer has a complex conditional

Comment on lines -115 to -129
boolean lookUpSubtypes = !this.options.contains(JacksonOption.SKIP_SUBTYPE_LOOKUP);
boolean includeTypeInfoTransform = !this.options.contains(JacksonOption.IGNORE_TYPE_INFO_TRANSFORM);
if (lookUpSubtypes || includeTypeInfoTransform) {
JsonSubTypesResolver subtypeResolver = new JsonSubTypesResolver(this.options);
if (lookUpSubtypes) {
generalConfigPart.withSubtypeResolver(subtypeResolver);
fieldConfigPart.withTargetTypeOverridesResolver(subtypeResolver::findTargetTypeOverrides);
methodConfigPart.withTargetTypeOverridesResolver(subtypeResolver::findTargetTypeOverrides);
}
if (includeTypeInfoTransform) {
generalConfigPart.withCustomDefinitionProvider(subtypeResolver);
fieldConfigPart.withCustomDefinitionProvider(subtypeResolver::provideCustomPropertySchemaDefinition);
methodConfigPart.withCustomDefinitionProvider(subtypeResolver::provideCustomPropertySchemaDefinition);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Complex Method
applyToConfigBuilder is no longer above the threshold for cyclomatic complexity

Comment on lines -303 to -304
private ObjectNode createSubtypeDefinition(ResolvedType javaType, JsonTypeInfo typeInfoAnnotation, JsonSubTypes subTypesAnnotation,
ObjectNode attributesToInclude, SchemaGenerationContext context) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Complex Method
createSubtypeDefinition is no longer above the threshold for cyclomatic complexity

Comment on lines -207 to -214
ObjectNode attributes;
if (scope instanceof FieldScope) {
attributes = AttributeCollector.collectFieldAttributes((FieldScope) scope, context);
} else if (scope instanceof MethodScope) {
attributes = AttributeCollector.collectMethodAttributes((MethodScope) scope, context);
} else {
attributes = null;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Complex Method
provideCustomPropertySchemaDefinition is no longer above the threshold for cyclomatic complexity

Comment on lines -303 to -304
private ObjectNode createSubtypeDefinition(ResolvedType javaType, JsonTypeInfo typeInfoAnnotation, JsonSubTypes subTypesAnnotation,
ObjectNode attributesToInclude, SchemaGenerationContext context) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Excess Number of Function Arguments
createSubtypeDefinition is no longer above the threshold for number of arguments

@@ -50,6 +50,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.BiPredicate;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Getting better: Overall Code Complexity
The mean cyclomatic complexity decreases from 5.64 to 5.26, threshold = 4

Why does this problem occur?

This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals. Read more.

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: FAILED

  • Declining Code Health: 1 findings(s) 🚩
  • Improving Code Health: 32 findings(s) ✅
  • Affected Hotspots: 2 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

Absence of Expected Change Pattern

  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/MethodScope.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/FieldScope.java

🚩 Declining Code Health (highest to lowest):

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 1 findings(s) 🚩
  • Improving Code Health: 33 findings(s) ✅
  • Affected Hotspots: 3 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

Absence of Expected Change Pattern

  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/MethodScope.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/FieldScope.java

🚩 Declining Code Health (highest to lowest):

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 0 findings(s) 🚩
  • Improving Code Health: 33 findings(s) ✅
  • Affected Hotspots: 3 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

Absence of Expected Change Pattern

  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/MethodScope.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/FieldScope.java

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 0 findings(s) 🚩
  • Improving Code Health: 33 findings(s) ✅
  • Affected Hotspots: 3 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

Absence of Expected Change Pattern

  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/MethodScope.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/FieldScope.java

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 0 findings(s) 🚩
  • Improving Code Health: 33 findings(s) ✅
  • Affected Hotspots: 3 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

Absence of Expected Change Pattern

  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/MethodScope.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/FieldScope.java

✅ Improving Code Health:

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Quality Gates: OK

  • Declining Code Health: 0 findings(s) 🚩
  • Improving Code Health: 33 findings(s) ✅
  • Affected Hotspots: 3 files(s) 🔥

Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene

Absence of Expected Change Pattern

  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
  • jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/MethodScope.java is usually changed with: jsonschema-generator/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/FieldScope.java

✅ Improving Code Health:

@CarstenWickner CarstenWickner marked this pull request as ready for review November 15, 2023 20:52
@CarstenWickner CarstenWickner merged commit 0daa090 into main Nov 15, 2023
11 checks passed
@CarstenWickner CarstenWickner deleted the code-health-refactor-3 branch November 15, 2023 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant