Skip to content

Commit 7a8e6fd

Browse files
jimbethancourtjimbethancourt
committed
#45 Completed implementation of Coupling Between Objects
- Completed implementation of the Coupling Between Objects ranker - Prioritizing starting with 1 to help users make better sense of priority ranking for God classes and Coupled Classes - Circle size is now max priority minus item priority - Going from green to red instead of blue to green for graph color - Upgraded version of dependency-check
1 parent 1f27fb1 commit 7a8e6fd

File tree

19 files changed

+411
-175
lines changed

19 files changed

+411
-175
lines changed

‎RefactorFirst_Sample_Report.png‎

35.3 KB
Loading

‎change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public File getGitDir(File basedir) {
4747
// and
4848
// https://github.com/centic9/jgit-cookbook/blob/master/src/main/java/org/dstadler/jgit/api/ReadFileFromCommit.java
4949
@Override
50-
public Map<String, ByteArrayOutputStream> (Repository repository) throws IOException {
50+
public Map<String, ByteArrayOutputStream> listRepositoryContentsAtHEAD(Repository repository) throws IOException {
5151
Ref head = repository.exactRef("HEAD");
5252
// a RevWalk allows us to walk over commits based on some filtering that is defined
5353
RevWalk walk = new RevWalk(repository);
@@ -56,7 +56,7 @@ public File getGitDir(File basedir) {
5656

5757
TreeWalk treeWalk = new TreeWalk(repository);
5858
treeWalk.addTree(tree);
59-
treeWalk.setRecursive(false);listRepositoryContentsAtHEAD
59+
treeWalk.setRecursive(false);
6060

6161
// TODO: extract rest of this method to test it
6262
Map<String, ByteArrayOutputStream> fileContentsCollection = new HashMap<>();

‎cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java‎

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
@Slf4j
1919
public class CostBenefitCalculator {
2020

21-
public List<RankedDisharmony> calculateCostBenefitValues(String repositoryPath) {
21+
Map<String, ByteArrayOutputStream> filesToScan = new HashMap<>();
22+
23+
public List<RankedDisharmony> calculateGodClassCostBenefitValues(String repositoryPath) {
2224

2325
RepositoryLogReader repositoryLogReader = new GitLogReader();
2426
Repository repository = null;
@@ -29,10 +31,7 @@ public List<RankedDisharmony> calculateCostBenefitValues(String repositoryPath)
2931
log.error("Failure to access Git repository", e);
3032
}
3133

32-
Map<String, ByteArrayOutputStream> filesToScan = getFilesToScan(repositoryLogReader, repository);
33-
34-
List<GodClass> godClasses = getGodClasses(filesToScan);
35-
List<CBOClass> cboClasses = getCBOClasses(filesToScan);
34+
List<GodClass> godClasses = getGodClasses(getFilesToScan(repositoryLogReader, repository));
3635

3736
List<ScmLogInfo> scmLogInfos = getRankedChangeProneness(repositoryLogReader, repository, godClasses);
3837

@@ -47,12 +46,12 @@ public List<RankedDisharmony> calculateCostBenefitValues(String repositoryPath)
4746
return rankedDisharmonies;
4847
}
4948

50-
List<ScmLogInfo> getRankedChangeProneness(
51-
RepositoryLogReader repositoryLogReader, Repository repository, List<GodClass> godClasses) {
49+
<T extends Disharmony> List<ScmLogInfo> getRankedChangeProneness(
50+
RepositoryLogReader repositoryLogReader, Repository repository, List<T> disharmonies) {
5251
List<ScmLogInfo> scmLogInfos = new ArrayList<>();
5352
log.info("Calculating Change Proneness for each God Class");
54-
for (GodClass godClass : godClasses) {
55-
String path = godClass.getFileName();
53+
for (Disharmony disharmony : disharmonies) {
54+
String path = disharmony.getFileName();
5655
ScmLogInfo scmLogInfo = null;
5756
try {
5857
scmLogInfo = repositoryLogReader.fileLog(repository, path);
@@ -87,10 +86,36 @@ private List<GodClass> getGodClasses(Map<String, ByteArrayOutputStream> filesToS
8786
return godClasses;
8887
}
8988

89+
public List<RankedDisharmony> calculateCBOCostBenefitValues(String repositoryPath) {
90+
91+
RepositoryLogReader repositoryLogReader = new GitLogReader();
92+
Repository repository = null;
93+
log.info("Initiating Cost Benefit calculation");
94+
try {
95+
repository = repositoryLogReader.gitRepository(new File(repositoryPath));
96+
} catch (IOException e) {
97+
log.error("Failure to access Git repository", e);
98+
}
99+
100+
List<CBOClass> cboClasses = getCBOClasses(getFilesToScan(repositoryLogReader, repository));
101+
102+
List<ScmLogInfo> scmLogInfos = getRankedChangeProneness(repositoryLogReader, repository, cboClasses);
103+
104+
Map<String, ScmLogInfo> rankedLogInfosByPath =
105+
scmLogInfos.stream().collect(Collectors.toMap(ScmLogInfo::getPath, logInfo -> logInfo, (a, b) -> b));
106+
107+
List<RankedDisharmony> rankedDisharmonies = new ArrayList<>();
108+
for (CBOClass cboClass : cboClasses) {
109+
rankedDisharmonies.add(new RankedDisharmony(cboClass, rankedLogInfosByPath.get(cboClass.getFileName())));
110+
}
111+
112+
return rankedDisharmonies;
113+
}
114+
90115
private List<CBOClass> getCBOClasses(Map<String, ByteArrayOutputStream> filesToScan) {
91116

92117
CBORuleRunner ruleRunner = new CBORuleRunner();
93-
118+
94119
log.info("Identifying highly coupled classes from files in repository");
95120
List<CBOClass> cboClasses = new ArrayList<>();
96121
for (Map.Entry<String, ByteArrayOutputStream> entry : filesToScan.entrySet()) {
@@ -105,10 +130,13 @@ private List<CBOClass> getCBOClasses(Map<String, ByteArrayOutputStream> filesToS
105130
return cboClasses;
106131
}
107132

108-
private Map<String, ByteArrayOutputStream> getFilesToScan(RepositoryLogReader repositoryLogReader, Repository repository) {
109-
Map<String, ByteArrayOutputStream> filesToScan = new HashMap<>();
133+
private Map<String, ByteArrayOutputStream> getFilesToScan(
134+
RepositoryLogReader repositoryLogReader, Repository repository) {
135+
110136
try {
111-
filesToScan = repositoryLogReader.listRepositoryContentsAtHEAD(repository);
137+
if (filesToScan.isEmpty()) {
138+
filesToScan = repositoryLogReader.listRepositoryContentsAtHEAD(repository);
139+
}
112140
} catch (IOException e) {
113141
log.error("Error reading Git repository contents", e);
114142
}

‎cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedDisharmony.java‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.time.Instant;
55
import lombok.Data;
66
import org.hjug.git.ScmLogInfo;
7+
import org.hjug.metrics.CBOClass;
78
import org.hjug.metrics.GodClass;
89

910
@Data
@@ -14,16 +15,15 @@ public class RankedDisharmony {
1415
private final String className;
1516
private final Integer effortRank;
1617
private final Integer changePronenessRank;
17-
private final Integer priority;
18+
private final Integer rawPriority;
19+
private Integer priority;
1820

19-
private Integer couplingCount;
20-
21-
private final Integer wmc;
22-
private final Integer wmcRank;
23-
private final Integer atfd;
24-
private final Integer atfdRank;
25-
private final Float tcc;
26-
private final Integer tccRank;
21+
private Integer wmc;
22+
private Integer wmcRank;
23+
private Integer atfd;
24+
private Integer atfdRank;
25+
private Float tcc;
26+
private Integer tccRank;
2727

2828
private final Instant firstCommitTime;
2929
private final Instant mostRecentCommitTime;
@@ -36,7 +36,7 @@ public RankedDisharmony(GodClass godClass, ScmLogInfo scmLogInfo) {
3636
className = godClass.getClassName();
3737
changePronenessRank = scmLogInfo.getChangePronenessRank();
3838
effortRank = godClass.getOverallRank();
39-
priority = changePronenessRank - effortRank;
39+
rawPriority = changePronenessRank - effortRank;
4040

4141
wmc = godClass.getWmc();
4242
wmcRank = godClass.getWmcRank();
@@ -49,4 +49,18 @@ public RankedDisharmony(GodClass godClass, ScmLogInfo scmLogInfo) {
4949
mostRecentCommitTime = Instant.ofEpochSecond(scmLogInfo.getMostRecentCommit());
5050
commitCount = scmLogInfo.getCommitCount();
5151
}
52+
53+
public RankedDisharmony(CBOClass cboClass, ScmLogInfo scmLogInfo) {
54+
path = scmLogInfo.getPath();
55+
// from https://stackoverflow.com/questions/1011287/get-file-name-from-a-file-location-in-java
56+
fileName = Paths.get(path).getFileName().toString();
57+
className = cboClass.getClassName();
58+
changePronenessRank = scmLogInfo.getChangePronenessRank();
59+
effortRank = cboClass.getCouplingCount();
60+
rawPriority = changePronenessRank - effortRank;
61+
62+
firstCommitTime = Instant.ofEpochSecond(scmLogInfo.getEarliestCommit());
63+
mostRecentCommitTime = Instant.ofEpochSecond(scmLogInfo.getMostRecentCommit());
64+
commitCount = scmLogInfo.getCommitCount();
65+
}
5266
}

‎cost-benefit-calculator/src/test/java/org/hjug/cbc/CostBenefitCalculatorTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ public void testCostBenefitCalculation() throws IOException, GitAPIException, In
5757
RevCommit secondCommit = git.commit().setMessage("message").call();
5858

5959
CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator();
60-
List<RankedDisharmony> disharmonies = costBenefitCalculator.calculateCostBenefitValues(
60+
List<RankedDisharmony> disharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues(
6161
git.getRepository().getDirectory().getPath());
6262

63-
Assert.assertEquals(0, disharmonies.get(0).getPriority().intValue());
64-
Assert.assertEquals(0, disharmonies.get(1).getPriority().intValue());
63+
Assert.assertEquals(0, disharmonies.get(0).getRawPriority().intValue());
64+
Assert.assertEquals(0, disharmonies.get(1).getRawPriority().intValue());
6565
}
6666

6767
@Test

‎effort-ranker/src/main/java/org/hjug/metrics/CBOClass.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
package org.hjug.metrics;
22

3-
import lombok.Data;
4-
53
import java.util.Scanner;
4+
import lombok.Data;
65

76
/**
87
* Created by Jim on 11/16/2016.
98
*/
109
@Data
11-
public class CBOClass {
10+
public class CBOClass implements Disharmony {
1211

1312
private String className;
1413
private String fileName;

‎effort-ranker/src/main/java/org/hjug/metrics/CBORuleRunner.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
package org.hjug.metrics;
22

3+
import java.io.File;
4+
import java.io.FileInputStream;
5+
import java.io.FileNotFoundException;
6+
import java.io.InputStream;
7+
import java.util.Optional;
38
import lombok.extern.slf4j.Slf4j;
49
import net.sourceforge.pmd.*;
510
import net.sourceforge.pmd.lang.Language;
611
import net.sourceforge.pmd.lang.LanguageRegistry;
712
import net.sourceforge.pmd.lang.java.JavaLanguageModule;
813
import org.hjug.metrics.rules.CBORule;
914

10-
import java.io.File;
11-
import java.io.FileInputStream;
12-
import java.io.FileNotFoundException;
13-
import java.io.InputStream;
14-
import java.util.Optional;
15-
1615
// based on http://sdoulger.blogspot.com/2010/12/call-pmd-from-your-code-with-you-custom.html
1716
@Slf4j
1817
public class CBORuleRunner {

‎effort-ranker/src/main/java/org/hjug/metrics/Disharmony.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package org.hjug.metrics;
2+
3+
public interface Disharmony {
4+
5+
String getFileName();
6+
}

‎effort-ranker/src/main/java/org/hjug/metrics/GodClass.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Created by Jim on 11/16/2016.
99
*/
1010
@Data
11-
public class GodClass {
11+
public class GodClass implements Disharmony {
1212

1313
private String className;
1414
private String fileName;

‎effort-ranker/src/main/java/org/hjug/metrics/rules/CBORule.java‎

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package org.hjug.metrics.rules;
22

3+
import java.util.HashSet;
4+
import java.util.List;
5+
import java.util.Set;
36
import net.sourceforge.pmd.lang.ast.Node;
47
import net.sourceforge.pmd.lang.java.ast.*;
58
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
@@ -9,18 +12,20 @@
912
import net.sourceforge.pmd.properties.PropertyFactory;
1013
import net.sourceforge.pmd.properties.constraints.NumericConstraints;
1114

12-
import java.util.HashSet;
13-
import java.util.List;
14-
import java.util.Set;
15-
1615
/**
1716
* Copy of PMD's CouplingBetweenObjectsRule
1817
* but generates the originally intended message containing coupling count
1918
*/
2019
public class CBORule extends AbstractJavaRule {
2120
private int couplingCount;
2221
private Set<String> typesFoundSoFar;
23-
private static final PropertyDescriptor<Integer> THRESHOLD_DESCRIPTOR = ((PropertyBuilder.GenericPropertyBuilder)((PropertyBuilder.GenericPropertyBuilder)((PropertyBuilder.GenericPropertyBuilder) PropertyFactory.intProperty("threshold").desc("Unique type reporting threshold")).require(NumericConstraints.positive())).defaultValue(20)).build();
22+
private static final PropertyDescriptor<Integer> THRESHOLD_DESCRIPTOR = ((PropertyBuilder.GenericPropertyBuilder)
23+
((PropertyBuilder.GenericPropertyBuilder)
24+
((PropertyBuilder.GenericPropertyBuilder) PropertyFactory.intProperty("threshold")
25+
.desc("Unique type reporting threshold"))
26+
.require(NumericConstraints.positive()))
27+
.defaultValue(20))
28+
.build();
2429

2530
public CBORule() {
2631
this.definePropertyDescriptor(THRESHOLD_DESCRIPTOR);
@@ -30,16 +35,19 @@ public Object visit(ASTCompilationUnit cu, Object data) {
3035
this.typesFoundSoFar = new HashSet();
3136
this.couplingCount = 0;
3237
Object returnObj = super.visit(cu, data);
33-
if (this.couplingCount > (Integer)this.getProperty(THRESHOLD_DESCRIPTOR)) {
34-
//only the line below is different from PMD's CouplingBetweenObjectsRule class
35-
this.addViolationWithMessage(data, cu, "A value of " + this.couplingCount + " may denote a high amount of coupling within the class");
38+
if (this.couplingCount > (Integer) this.getProperty(THRESHOLD_DESCRIPTOR)) {
39+
// only the line below is different from PMD's CouplingBetweenObjectsRule class
40+
this.addViolationWithMessage(
41+
data,
42+
cu,
43+
"A value of " + this.couplingCount + " may denote a high amount of coupling within the class");
3644
}
3745

3846
return returnObj;
3947
}
4048

4149
public Object visit(ASTResultType node, Object data) {
42-
for(int x = 0; x < node.getNumChildren(); ++x) {
50+
for (int x = 0; x < node.getNumChildren(); ++x) {
4351
Node tNode = node.getChild(x);
4452
if (tNode instanceof ASTType) {
4553
Node reftypeNode = tNode.getChild(0);
@@ -66,10 +74,10 @@ public Object visit(ASTFormalParameter node, Object data) {
6674
}
6775

6876
public Object visit(ASTFieldDeclaration node, Object data) {
69-
for(int x = 0; x < node.getNumChildren(); ++x) {
77+
for (int x = 0; x < node.getNumChildren(); ++x) {
7078
Node firstStmt = node.getChild(x);
7179
if (firstStmt instanceof ASTType) {
72-
ASTType tp = (ASTType)firstStmt;
80+
ASTType tp = (ASTType) firstStmt;
7381
Node nd = tp.getChild(0);
7482
this.checkVariableType(nd, nd.getImage());
7583
}
@@ -79,35 +87,49 @@ public Object visit(ASTFieldDeclaration node, Object data) {
7987
}
8088

8189
private void handleASTTypeChildren(Node node) {
82-
for(int x = 0; x < node.getNumChildren(); ++x) {
90+
for (int x = 0; x < node.getNumChildren(); ++x) {
8391
Node sNode = node.getChild(x);
8492
if (sNode instanceof ASTType) {
8593
Node nameNode = sNode.getChild(0);
8694
this.checkVariableType(nameNode, nameNode.getImage());
8795
}
8896
}
89-
9097
}
9198

9299
private void checkVariableType(Node nameNode, String variableType) {
93-
List<ASTClassOrInterfaceDeclaration> parentTypes = nameNode.getParentsOfType(ASTClassOrInterfaceDeclaration.class);
100+
List<ASTClassOrInterfaceDeclaration> parentTypes =
101+
nameNode.getParentsOfType(ASTClassOrInterfaceDeclaration.class);
94102
if (!parentTypes.isEmpty()) {
95-
if (!((ASTClassOrInterfaceDeclaration)parentTypes.get(0)).isInterface()) {
96-
ClassScope clzScope = (ClassScope)((JavaNode)nameNode).getScope().getEnclosingScope(ClassScope.class);
97-
if (!clzScope.getClassName().equals(variableType) && !this.filterTypes(variableType) && !this.typesFoundSoFar.contains(variableType)) {
103+
if (!((ASTClassOrInterfaceDeclaration) parentTypes.get(0)).isInterface()) {
104+
ClassScope clzScope =
105+
(ClassScope) ((JavaNode) nameNode).getScope().getEnclosingScope(ClassScope.class);
106+
if (!clzScope.getClassName().equals(variableType)
107+
&& !this.filterTypes(variableType)
108+
&& !this.typesFoundSoFar.contains(variableType)) {
98109
++this.couplingCount;
99110
this.typesFoundSoFar.add(variableType);
100111
}
101-
102112
}
103113
}
104114
}
105115

106116
private boolean filterTypes(String variableType) {
107-
return variableType != null && (variableType.startsWith("java.lang.") || "String".equals(variableType) || this.filterPrimitivesAndWrappers(variableType));
117+
return variableType != null
118+
&& (variableType.startsWith("java.lang.")
119+
|| "String".equals(variableType)
120+
|| this.filterPrimitivesAndWrappers(variableType));
108121
}
109122

110123
private boolean filterPrimitivesAndWrappers(String variableType) {
111-
return "int".equals(variableType) || "Integer".equals(variableType) || "char".equals(variableType) || "Character".equals(variableType) || "double".equals(variableType) || "long".equals(variableType) || "short".equals(variableType) || "float".equals(variableType) || "byte".equals(variableType) || "boolean".equals(variableType);
124+
return "int".equals(variableType)
125+
|| "Integer".equals(variableType)
126+
|| "char".equals(variableType)
127+
|| "Character".equals(variableType)
128+
|| "double".equals(variableType)
129+
|| "long".equals(variableType)
130+
|| "short".equals(variableType)
131+
|| "float".equals(variableType)
132+
|| "byte".equals(variableType)
133+
|| "boolean".equals(variableType);
112134
}
113135
}

0 commit comments

Comments
 (0)