Skip to content

Commit

Permalink
#33 "source code style rule definition"
Browse files Browse the repository at this point in the history
- immutable types with collections with defensive copies and usage of Collections.unmodifiableXzy(...)
  • Loading branch information
janScheible committed Jul 4, 2022
1 parent 76098ba commit a9ab48d
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 46 deletions.
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ Unresolvable methods (no coverage information available):

## Source code style

1. avoid static methods for easier testing
1. avoid `static` methods for easier testing
1. exception: (package) private helper methods in classes that are static to enforce functional purity
1. exception: completely stateless `*Utils` classes with static methods only
1. utility classes must be `abstract` and have a private default constructor
Expand All @@ -222,3 +222,6 @@ Unresolvable methods (no coverage information available):
1. (package private) constructor in model class with `BuilderImpl` as only parameter
1. inner step interfaces in `<ModelClassName>Builder` with `Step` suffix
1. inner static class `BuilderImpl` implementing all the steps
1. usage of immutable data structures only
1. for collections defensive copies and `Collections.unmodifiableXzy(...)` in constructor
1. other fields or elements in collections have to be immutable
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.scheible.testgapanalysis.analysis;

import static java.util.Collections.unmodifiableMap;
import static java.util.Collections.unmodifiableSet;

import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

Expand All @@ -26,12 +26,12 @@ public AnalysisResult(final Map<ParsedMethod, MethodWithCoverageInfo> coveredMet
final Map<ParsedMethod, MethodWithCoverageInfo> uncoveredMethods, final Set<ParsedMethod> emptyMethods,
final Set<ParsedMethod> unresolvableMethods,
final Map<MethodWithCoverageInfo, Set<ParsedMethod>> ambiguouslyResolvedCoverage) {
this.coveredMethods = unmodifiableMap(coveredMethods);
this.uncoveredMethods = unmodifiableMap(uncoveredMethods);
this.coveredMethods = Collections.unmodifiableMap(new HashMap<>(coveredMethods));
this.uncoveredMethods = Collections.unmodifiableMap(new HashMap<>(uncoveredMethods));

this.emptyMethods = unmodifiableSet(emptyMethods);
this.unresolvableMethods = unmodifiableSet(unresolvableMethods);
this.ambiguouslyResolvedCoverage = unmodifiableMap(ambiguouslyResolvedCoverage);
this.emptyMethods = Collections.unmodifiableSet(new HashSet<>(emptyMethods));
this.unresolvableMethods = Collections.unmodifiableSet(new HashSet<>(unresolvableMethods));
this.ambiguouslyResolvedCoverage = Collections.unmodifiableMap(new HashMap<>(ambiguouslyResolvedCoverage));
}

public Map<ParsedMethod, MethodWithCoverageInfo> getCoveredMethods() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.scheible.testgapanalysis.analysis.testgap;

import static java.util.Collections.unmodifiableMap;
import static java.util.Collections.unmodifiableSet;

import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -179,25 +179,26 @@ public static WorkDirStep builder() {
this.newCommitHash = builder.newCommitHash.orElse(null);
compareWithWorkingCopyChanges = builder.newCommitHash.isPresent() ? null : Boolean.TRUE;

this.jaCoCoReportFiles = unmodifiableSet(builder.jaCoCoReportFiles);
this.jaCoCoReportFiles = Collections.unmodifiableSet(new HashSet<>(builder.jaCoCoReportFiles));
this.jaCoCoCoverageCount = builder.jaCoCoCoverageCount;

this.newOrChangedFiles = unmodifiableSet(builder.newOrChangedFiles);
this.newOrChangedFiles = Collections.unmodifiableSet(new HashSet<>(builder.newOrChangedFiles));
consideredNewOrChangedFilesCount = (int) builder.newOrChangedFiles.stream().filter(f -> !f.isSkipped()).count();

coveredMethodsCount = builder.coveredMethods.size();
this.coveredMethods = unmodifiableSet(builder.coveredMethods);
this.coveredMethods = Collections.unmodifiableSet(new HashSet<>(builder.coveredMethods));
uncoveredMethodsCount = builder.uncoveredMethods.size();
this.uncoveredMethods = unmodifiableSet(builder.uncoveredMethods);
this.uncoveredMethods = Collections.unmodifiableSet(new HashSet<>(builder.uncoveredMethods));
coverageRatio = coveredMethodsCount + uncoveredMethodsCount > 0
? (double) coveredMethodsCount / (coveredMethodsCount + uncoveredMethodsCount)
: 1.0;
emptyMethodsCount = builder.emptyMethods.size();
this.emptyMethods = unmodifiableSet(builder.emptyMethods);
this.emptyMethods = Collections.unmodifiableSet(new HashSet<>(builder.emptyMethods));
unresolvableMethodsCount = builder.unresolvableMethods.size();
this.unresolvableMethods = unmodifiableSet(builder.unresolvableMethods);
this.unresolvableMethods = Collections.unmodifiableSet(new HashSet<>(builder.unresolvableMethods));
ambiguouslyResolvedCount = builder.ambiguouslyResolvedCoverage.size();
this.ambiguouslyResolvedCoverage = unmodifiableMap(builder.ambiguouslyResolvedCoverage);
this.ambiguouslyResolvedCoverage = Collections
.unmodifiableMap(new HashMap<>(builder.ambiguouslyResolvedCoverage));
}

public String getWorkDir() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.scheible.testgapanalysis.debug;

import static java.util.Collections.unmodifiableMap;
import static java.util.Collections.unmodifiableSet;

import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -33,14 +33,14 @@ public static CoverageInfoCountStep builder() {

DebugCoverageResolutionReport(final BuilderImpl builder) {
this.coverageInfoCount = builder.coverageInfoCount;
this.jaCoCoReportFiles = unmodifiableSet(builder.jaCoCoReportFiles);
this.jaCoCoReportFiles = Collections.unmodifiableSet(new HashSet<>(builder.jaCoCoReportFiles));
this.javaFileCount = builder.javaFileCount;

this.resolved = unmodifiableMap(builder.resolved);
this.empty = unmodifiableSet(builder.empty);
this.resolved = Collections.unmodifiableMap(new HashMap<>(builder.resolved));
this.empty = Collections.unmodifiableSet(new HashSet<>(builder.empty));

this.unresolved = unmodifiableSet(builder.unresolved);
this.ambiguousCoverage = unmodifiableMap(builder.ambiguousCoverage);
this.unresolved = Collections.unmodifiableSet(new HashSet<>(builder.unresolved));
this.ambiguousCoverage = Collections.unmodifiableMap(new HashMap<>(builder.ambiguousCoverage));
}

public int getCoverageInfoCount() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.scheible.testgapanalysis.git;

import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand All @@ -26,11 +28,11 @@ public class RepositoryStatus {
this.oldCommitHash = oldCommitHash;
this.newCommitHash = newCommitHash;

this.addedFiles = Collections.unmodifiableSet(addedFiles);
this.changedFiles = Collections.unmodifiableSet(changedFiles);
this.addedFiles = Collections.unmodifiableSet(new HashSet<>(addedFiles));
this.changedFiles = Collections.unmodifiableSet(new HashSet<>(changedFiles));

this.oldContents = Collections.unmodifiableMap(oldContents);
this.newContents = Collections.unmodifiableMap(newContents);
this.oldContents = Collections.unmodifiableMap(new HashMap<>(oldContents));
this.newContents = Collections.unmodifiableMap(new HashMap<>(newContents));
}

public String getOldCommitHash() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.scheible.testgapanalysis.jacoco.resolver;

import static java.util.Collections.unmodifiableMap;
import static java.util.Collections.unmodifiableSet;

import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -71,19 +69,19 @@ private static Map<MethodWithCoverageInfo, Set<ParsedMethod>> findAmbiguouslyRes
}

public Map<ParsedMethod, MethodWithCoverageInfo> getResolvedMethods() {
return unmodifiableMap(resolvedMethods);
return Collections.unmodifiableMap(resolvedMethods);
}

public Set<ParsedMethod> getEmptyMethods() {
return unmodifiableSet(emptyMethods);
return Collections.unmodifiableSet(emptyMethods);
}

public Set<ParsedMethod> getUnresolvedMethods() {
return unmodifiableSet(unresolvedMethods);
return Collections.unmodifiableSet(unresolvedMethods);
}

public Map<MethodWithCoverageInfo, Set<ParsedMethod>> getAmbiguousCoverage() {
return unmodifiableMap(ambiguousCoverage);
return Collections.unmodifiableMap(ambiguousCoverage);
}

public boolean contains(final ParsedMethod method) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.scheible.testgapanalysis.parser;

import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableList;
import static java.util.Collections.unmodifiableMap;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -48,20 +49,22 @@ public static MethodTypeStep builder() {
this.methodType = builder.methodType;
this.topLevelTypeFqn = builder.topLevelTypeFqn;
this.topLevelSimpleName = JavaMethodUtils.getSimpleName(builder.topLevelTypeFqn, ".");
this.scope = unmodifiableList(builder.scope);
this.scope = Collections.unmodifiableList(new ArrayList<>(builder.scope));
this.enclosingSimpleName = scope.isEmpty() ? topLevelSimpleName : builder.scope.get(builder.scope.size() - 1);
this.name = builder.name;
this.relevantCode = builder.relevantCode;
this.codeLines = unmodifiableList(builder.codeLines.stream().sorted().collect(Collectors.toList()));
this.codeLines = Collections
.unmodifiableList(new ArrayList<>(builder.codeLines.stream().sorted().collect(Collectors.toList())));
this.codeColumn = builder.codeColumn;
this.empty = builder.empty;

this.argumentTypes = builder.argumentTypes != null
? unmodifiableList(builder.argumentTypes)
: IntStream.range(0, builder.argumentCount).boxed().map(i -> "Object").collect(Collectors.toList());
this.argumentTypes = Collections.unmodifiableList(builder.argumentTypes != null
? new ArrayList<>(builder.argumentTypes)
: IntStream.range(0, builder.argumentCount).boxed().map(i -> "Object").collect(Collectors.toList()));

argumentCount = argumentTypes.size();
this.typeParameters = builder.typeParameters != null ? unmodifiableMap(builder.typeParameters) : emptyMap();
this.typeParameters = Collections
.unmodifiableMap(builder.typeParameters != null ? new HashMap<>(builder.typeParameters) : emptyMap());
this.outerDeclaringType = builder.outerDeclaringType.orElse(null);
}

Expand Down

0 comments on commit a9ab48d

Please sign in to comment.