From 91773c8a8b07bce50b9b35105b96251de74fcc91 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Sun, 14 Jun 2026 09:03:04 -0500 Subject: [PATCH 1/9] #185 Moving and renaming things to prepare for package cycle analysis --- .../main/java/org/hjug/cbc/CycleRanker.java | 23 ++++--- .../mavenreport/RefactorFirstMavenReport.java | 1 - .../report/SimpleHtmlReport.java | 65 ++++++++----------- 3 files changed, 37 insertions(+), 52 deletions(-) diff --git a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java index c94cd77e..8115f3ae 100644 --- a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java +++ b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java @@ -19,30 +19,25 @@ public class CycleRanker { private final String repositoryPath; - @Getter - private Graph classReferencesGraph; - @Getter private CodebaseGraphDTO codebaseGraphDTO; - public void generateClassReferencesGraph(boolean excludeTests, String testSourceDirectory) { + public CodebaseGraphDTO generateClassReferencesGraph(boolean excludeTests, String testSourceDirectory) { try { JavaGraphBuilder javaGraphBuilder = new JavaGraphBuilder(); - codebaseGraphDTO = javaGraphBuilder.getCodebaseGraphDTO(repositoryPath, excludeTests, testSourceDirectory); - - classReferencesGraph = codebaseGraphDTO.getClassReferencesGraph(); } catch (IOException e) { throw new RuntimeException(e); } + + return codebaseGraphDTO; } - public List performCycleAnalysis(boolean excludeTests, String testSourceDirectory) { - List rankedCycles = new ArrayList<>(); + public List performCycleAnalysis(Graph graph) { + List rankedCycles; try { boolean calculateCycleChurn = false; - generateClassReferencesGraph(excludeTests, testSourceDirectory); - identifyRankedCycles(rankedCycles); + rankedCycles = new ArrayList<>(identifyRankedCycles(graph)); sortRankedCycles(rankedCycles, calculateCycleChurn); setPriorities(rankedCycles); } catch (IOException e) { @@ -52,7 +47,9 @@ public List performCycleAnalysis(boolean excludeTests, String testS return rankedCycles; } - private void identifyRankedCycles(List rankedCycles) throws IOException { + private List identifyRankedCycles(Graph classReferencesGraph) + throws IOException { + List rankedCycles = new ArrayList<>(); CircularReferenceChecker circularReferenceChecker = new CircularReferenceChecker<>(); Map> cycles = @@ -65,6 +62,8 @@ private void identifyRankedCycles(List rankedCycles) throws IOExcep rankedCycles.add(createRankedCycle(vertex, subGraph, cycleNodes, 0.0, new HashSet<>())); }); + + return rankedCycles; } public CycleNode classToCycleNode(String fqnClass) { diff --git a/refactor-first-maven-plugin/src/main/java/org/hjug/mavenreport/RefactorFirstMavenReport.java b/refactor-first-maven-plugin/src/main/java/org/hjug/mavenreport/RefactorFirstMavenReport.java index 8fb8dff5..ed814b00 100644 --- a/refactor-first-maven-plugin/src/main/java/org/hjug/mavenreport/RefactorFirstMavenReport.java +++ b/refactor-first-maven-plugin/src/main/java/org/hjug/mavenreport/RefactorFirstMavenReport.java @@ -1,7 +1,6 @@ package org.hjug.mavenreport; import java.util.*; - import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import org.apache.maven.doxia.markup.HtmlMarkup; diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java index b17a8739..c0537b65 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java @@ -50,7 +50,7 @@ public class SimpleHtmlReport { public final String[] classCycleTableHeadings = {"Classes", "Relationships"}; Graph classGraph; - Map> cycles; + Map> classCycles; Set vertexesToRemove = Set.of(); // initialize for unit tests Set edgesToRemove = Set.of(); @@ -173,19 +173,23 @@ public StringBuilder generateReport( } CycleRanker cycleRanker = new CycleRanker(projectBaseDir); - List rankedCycles = List.of(); + List rankedClassCycles = List.of(); + CodebaseGraphDTO codebaseGraphDTO; if (analyzeCycles) { log.info("Analyzing Cycles"); - rankedCycles = cycleRanker.performCycleAnalysis(excludeTests, testSourceDirectory); - } else { cycleRanker.generateClassReferencesGraph(excludeTests, testSourceDirectory); + codebaseGraphDTO = cycleRanker.getCodebaseGraphDTO(); + rankedClassCycles = cycleRanker.performCycleAnalysis(codebaseGraphDTO.getClassReferencesGraph()); + } else { + codebaseGraphDTO = cycleRanker.generateClassReferencesGraph(excludeTests, testSourceDirectory); } - classGraph = cycleRanker.getClassReferencesGraph(); - cycles = new CircularReferenceChecker().getCycles(classGraph); + classGraph = codebaseGraphDTO.getClassReferencesGraph(); + classCycles = new CircularReferenceChecker().getCycles(classGraph); + Map edgeCycleCounts = new HashMap<>(); // Skip vertex and edge removal analysis if there are no cycles - if (!cycles.isEmpty()) { + if (!classCycles.isEmpty()) { // Identify vertexes to remove log.info("Identifying vertexes to remove"); EnhancedParameterComputer enhancedParameterComputer = @@ -207,36 +211,19 @@ public StringBuilder generateReport( PageRankFAS pageRankFAS = new PageRankFAS<>(classGraph, new SuperTypeToken<>() {}); edgesToRemove = pageRankFAS.computeFeedbackArcSet(); - } - // capture the number of cycles each edge to remove is in - Map edgeToRemoveCycleCounts = new HashMap<>(); - for (DefaultWeightedEdge edgeToRemove : edgesToRemove) { - int cycleCount = 0; - for (AsSubgraph cycle : cycles.values()) { - if (cycle.containsEdge(edgeToRemove)) { - cycleCount++; + // capture the number of cycles each edge to remove is in + for (DefaultWeightedEdge edgeToRemove : edgesToRemove) { + int cycleCount = 0; + for (AsSubgraph cycle : classCycles.values()) { + if (cycle.containsEdge(edgeToRemove)) { + cycleCount++; + } } + edgeCycleCounts.put(edgeToRemove, cycleCount); } - edgeToRemoveCycleCounts.put(edgeToRemove, cycleCount); } - // int edgeWeight = (int) classGraph.getEdgeWeight(defaultWeightedEdge); - // map sources to CycleNodes to get paths and get churn in try/finally block below - Map sourceNodeInfos = new HashMap<>(); - Map targetNodeInfos = new HashMap<>(); - for (DefaultWeightedEdge defaultWeightedEdge : edgesToRemove) { - String edgeSource = classGraph.getEdgeSource(defaultWeightedEdge); - CycleNode sourceNode = cycleRanker.classToCycleNode(edgeSource); - sourceNodeInfos.put(defaultWeightedEdge, sourceNode); - - String edgeTarget = classGraph.getEdgeTarget(defaultWeightedEdge); - CycleNode targetNode = cycleRanker.classToCycleNode(edgeTarget); - targetNodeInfos.put(defaultWeightedEdge, targetNode); - } - - List edgeDisharmonies = List.of(); - // Ordered (type, anchorId, displayTitle, isMethodLevel) for all disharmonies final List disharmonySpecs = List.of( new DisharmonySpec(DisharmonyTypes.GOD_CLASS, "GOD", "God Classes", false), @@ -259,11 +246,11 @@ public StringBuilder generateReport( log.info("Identifying Object Oriented Disharmonies"); - CodebaseGraphDTO codebaseGraphDTO = cycleRanker.getCodebaseGraphDTO(); + List edgeDisharmonies = List.of(); try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir, codebaseGraphDTO.getClassToSourceFilePathMapping())) { edgeDisharmonies = costBenefitCalculator.calculateSourceNodeCostBenefitValues( - classGraph, edgeToRemoveCycleCounts, codebaseGraphDTO, vertexesToRemove); + classGraph, edgeCycleCounts, codebaseGraphDTO, vertexesToRemove); for (DisharmonySpec spec : disharmonySpecs) { List instances = spec.methodLevel() @@ -287,7 +274,7 @@ public StringBuilder generateReport( // - Provide guidance on where to move the method if one is in the list to remove boolean hasAnyDisharmony = - !edgesToRemove.isEmpty() || !rankedCycles.isEmpty() || !rankedDisharmoniesByAnchor.isEmpty(); + !edgesToRemove.isEmpty() || !rankedClassCycles.isEmpty() || !rankedDisharmoniesByAnchor.isEmpty(); String repoUrl; try (GitLogReader glr = new GitLogReader(new File(projectBaseDir))) { @@ -308,7 +295,7 @@ public StringBuilder generateReport( } stringBuilder.append("
\n" + "\n" + "
\n"); log.info("Generating HTML Report"); @@ -332,8 +319,8 @@ public StringBuilder generateReport( } } - if (!rankedCycles.isEmpty()) { - stringBuilder.append(renderCycles(rankedCycles, repoUrl, codebaseGraphDTO)); + if (!rankedClassCycles.isEmpty()) { + stringBuilder.append(renderCycles(rankedClassCycles, repoUrl, codebaseGraphDTO)); } log.debug(stringBuilder.toString()); @@ -394,7 +381,7 @@ private String renderEdgeDisharmonies( "\n"); stringBuilder.append("

Refactor Starting with Priority 1

\n"); stringBuilder.append("
\n"); - stringBuilder.append("Current Cycle Count: ").append(cycles.size()).append("
\n"); + stringBuilder.append("Current Cycle Count: ").append(classCycles.size()).append("
\n"); stringBuilder .append("Number of Relationships to Remove: ") From 8dc9dcf01689a8cf7c1310d811776f19cf83d30b Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Tue, 16 Jun 2026 19:10:04 -0500 Subject: [PATCH 2/9] #185 Now rendering package cycle table! Now rendering package cycle table! --- .../main/java/org/hjug/git/GitLogReader.java | 2 +- .../GraphDependencyCollector.java | 10 + .../visitor/BaseTypeProcessor.java | 8 - .../org/hjug/cbc/CostBenefitCalculator.java | 7 +- .../main/java/org/hjug/cbc/CycleRanker.java | 31 +- .../main/java/org/hjug/cbc/RankedCycle.java | 62 +--- .../java/org/hjug/cbc/RankedDisharmony.java | 10 +- .../hjug/cbc/CostBenefitCalculatorTest.java | 26 +- .../hjug/feedback/CycleRemovalComputer.java | 60 ++++ .../org/hjug/feedback/CycleRemovalResult.java | 15 + .../hjug/refactorfirst/report/HtmlReport.java | 10 +- .../report/SimpleHtmlReport.java | 300 ++++++++++++------ .../refactorfirst/report/HtmlReportTest.java | 3 +- 13 files changed, 327 insertions(+), 217 deletions(-) create mode 100644 graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalComputer.java create mode 100644 graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalResult.java diff --git a/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java b/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java index 5b1c839d..3c6afeed 100644 --- a/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java +++ b/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java @@ -51,7 +51,7 @@ public void close() throws Exception { git.close(); } - public File getGitDir(File basedir) { + public static File getGitDir(File basedir) { FileRepositoryBuilder repositoryBuilder = new FileRepositoryBuilder().findGitDir(basedir); return repositoryBuilder.getGitDir(); } diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java index ca15faf0..73523f96 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java @@ -39,6 +39,8 @@ public void addClassDependency(String fromClassFqn, String toClassFqn) { DefaultWeightedEdge edge = classReferencesGraph.getEdge(fromClassFqn, toClassFqn); classReferencesGraph.setEdgeWeight(edge, classReferencesGraph.getEdgeWeight(edge) + 1); } + + addPackageDependency(getPackageFromFqn(fromClassFqn), getPackageFromFqn(toClassFqn)); } @Override @@ -58,6 +60,14 @@ public void addPackageDependency(String fromPackageName, String toPackageName) { } } + protected String getPackageFromFqn(String fqn) { + if (!fqn.contains(".")) { + return ""; + } + int lastIndex = fqn.lastIndexOf("."); + return fqn.substring(0, lastIndex); + } + @Override public void recordClassLocation(String classFqn, String sourceFilePath) { // This will be handled by JavaVisitor which maintains the mapping diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/visitor/BaseTypeProcessor.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/visitor/BaseTypeProcessor.java index f4affd0e..b33c7bed 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/visitor/BaseTypeProcessor.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/visitor/BaseTypeProcessor.java @@ -65,12 +65,4 @@ protected void processAnnotations(String ownerFqn, Cursor cursor) { processAnnotation(ownerFqn, annotation, cursor); } } - - protected String getPackageFromFqn(String fqn) { - if (!fqn.contains(".")) { - return ""; - } - int lastIndex = fqn.lastIndexOf("."); - return fqn.substring(0, lastIndex); - } } diff --git a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java index e8acf4b6..7e889f38 100644 --- a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java +++ b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java @@ -326,7 +326,7 @@ private List getCBOClasses() { return cboClasses; } - public List calculateSourceNodeCostBenefitValues( + public List calculateRelationshipCostBenefitValues( Graph classGraph, Map edgeToRemoveCycleCounts, CodebaseGraphDTO dto, @@ -352,6 +352,7 @@ public List calculateSourceNodeCostBenefitValues( targetNodeShouldBeRemoved, dto.getClassDisharmonyCountForClass(edgeSource), dto.getClassDisharmonyCountForClass(edgeTarget)); + edgesThatNeedToBeRemoved.add(edgeThatNeedsToBeRemoved); } @@ -383,8 +384,8 @@ static void sortEdgesThatNeedToBeRemoved(List rankedDisharmoni // then by weight, with lowest weight edges bubbling to the top .thenComparingInt(RankedDisharmony::getEffortRank) // then by disharmony count - .thenComparingInt(RankedDisharmony::getChangePronenessRank) - .thenComparingInt(RankedDisharmony::getEdgeTargetChangePronenessRank) + .thenComparingInt(RankedDisharmony::getEdgeSourceDisharmonyCount) + .thenComparingInt(RankedDisharmony::getEdgeTargetDisharmonyCount) // then if the source node is in the list of nodes to be removed // multiplying by -1 reverses the sort order (reverse doesn't work in chained comparators) .thenComparingInt(rankedDisharmony -> -1 * rankedDisharmony.getSourceNodeShouldBeRemoved()) diff --git a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java index 8115f3ae..23d0117d 100644 --- a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java +++ b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java @@ -22,6 +22,7 @@ public class CycleRanker { @Getter private CodebaseGraphDTO codebaseGraphDTO; + // TODO: should this method belong in this class? public CodebaseGraphDTO generateClassReferencesGraph(boolean excludeTests, String testSourceDirectory) { try { JavaGraphBuilder javaGraphBuilder = new JavaGraphBuilder(); @@ -33,12 +34,10 @@ public CodebaseGraphDTO generateClassReferencesGraph(boolean excludeTests, Strin return codebaseGraphDTO; } - public List performCycleAnalysis(Graph graph) { + public List rankCycles(Graph graph) { List rankedCycles; try { - boolean calculateCycleChurn = false; rankedCycles = new ArrayList<>(identifyRankedCycles(graph)); - sortRankedCycles(rankedCycles, calculateCycleChurn); setPriorities(rankedCycles); } catch (IOException e) { throw new RuntimeException(e); @@ -60,7 +59,7 @@ private List identifyRankedCycles(Graph log.info(cycleNode.toString())) .collect(Collectors.toList()); - rankedCycles.add(createRankedCycle(vertex, subGraph, cycleNodes, 0.0, new HashSet<>())); + rankedCycles.add(new RankedCycle(vertex, subGraph.vertexSet(), subGraph.edgeSet(), cycleNodes)); }); return rankedCycles; @@ -81,30 +80,8 @@ private String getClassRepoPath(String classInCycle) { return fileRepoPath; } - private RankedCycle createRankedCycle( - String vertex, - AsSubgraph subGraph, - List cycleNodes, - double minCut, - Set minCutEdges) { - - return new RankedCycle(vertex, subGraph.vertexSet(), subGraph.edgeSet(), minCut, minCutEdges, cycleNodes); - } - - private static void sortRankedCycles(List rankedCycles, boolean calculateChurnForCycles) { - if (calculateChurnForCycles) { - rankedCycles.sort(Comparator.comparing(RankedCycle::getAverageChangeProneness)); - - int cpr = 1; - for (RankedCycle rankedCycle : rankedCycles) { - rankedCycle.setChangePronenessRank(cpr++); - } - } else { - rankedCycles.sort(Comparator.comparing(RankedCycle::getRawPriority).reversed()); - } - } - private static void setPriorities(List rankedCycles) { + rankedCycles.sort(Comparator.comparing(RankedCycle::getRawPriority).reversed()); int priority = 1; for (RankedCycle rankedCycle : rankedCycles) { rankedCycle.setPriority(priority++); diff --git a/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedCycle.java b/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedCycle.java index 54b65ba6..b049b079 100644 --- a/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedCycle.java +++ b/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedCycle.java @@ -1,6 +1,5 @@ package org.hjug.cbc; -import java.util.HashSet; import java.util.List; import java.util.Set; import lombok.Data; @@ -12,76 +11,19 @@ public class RankedCycle { private final String cycleName; - private Integer changePronenessRankSum = 0; private final Set vertexSet; private final Set edgeSet; - private final double minCutCount; - private final Set minCutEdges; private final List cycleNodes; - private float rawPriority; private Integer priority = 0; - private float averageChangeProneness; - private Integer changePronenessRank = 0; - private float impact; - - public RankedCycle( - String cycleName, - Set vertexSet, - Set edgeSet, - double minCutCount, - Set minCutEdges, - List cycleNodes) { - this.cycleNodes = cycleNodes; - this.cycleName = cycleName; - this.vertexSet = vertexSet; - this.edgeSet = edgeSet; - this.minCutCount = minCutCount; - - if (null == minCutEdges) { - this.minCutEdges = new HashSet<>(); - } else { - this.minCutEdges = minCutEdges; - } - - if (minCutCount == 0.0) { - this.impact = (float) (vertexSet.size()); - } else { - this.impact = (float) (vertexSet.size() / minCutCount); - } - - this.rawPriority = this.impact; - } public RankedCycle( - String cycleName, - Integer changePronenessRankSum, - Set vertexSet, - Set edgeSet, - double minCutCount, - Set minCutEdges, - List cycleNodes) { + String cycleName, Set vertexSet, Set edgeSet, List cycleNodes) { this.cycleNodes = cycleNodes; this.cycleName = cycleName; - this.changePronenessRankSum = changePronenessRankSum; this.vertexSet = vertexSet; this.edgeSet = edgeSet; - this.minCutCount = minCutCount; - - if (null == minCutEdges) { - this.minCutEdges = new HashSet<>(); - } else { - this.minCutEdges = minCutEdges; - } - - if (minCutCount == 0.0) { - this.impact = (float) (vertexSet.size()); - } else { - this.impact = (float) (vertexSet.size() / minCutCount); - } - - this.averageChangeProneness = (float) changePronenessRankSum / vertexSet.size(); - this.rawPriority = this.impact + averageChangeProneness; + this.rawPriority = (float) (vertexSet.size()); // go away? } } diff --git a/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedDisharmony.java b/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedDisharmony.java index 797b9086..aea907a8 100644 --- a/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedDisharmony.java +++ b/cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedDisharmony.java @@ -22,7 +22,7 @@ public class RankedDisharmony { private String fileName; private final String className; private final Integer effortRank; - private final Integer changePronenessRank; + private Integer changePronenessRank; private Integer rawPriority; private Integer priority = 0; @@ -44,7 +44,8 @@ public class RankedDisharmony { private int sourceNodeShouldBeRemoved; private int targetNodeShouldBeRemoved; private String edgeTargetClass; - private Integer edgeTargetChangePronenessRank; + private Integer edgeSourceDisharmonyCount; + private Integer edgeTargetDisharmonyCount; public RankedDisharmony(GodClass godClass, ScmLogInfo scmLogInfo) { path = scmLogInfo.getPath(); @@ -100,6 +101,7 @@ public RankedDisharmony(DisharmonyInstance instance, ScmLogInfo scmLogInfo) { commitCount = scmLogInfo.getCommitCount(); } + // TODO: Move to a separate class? public RankedDisharmony( String edgeSource, DefaultWeightedEdge edge, @@ -113,8 +115,8 @@ public RankedDisharmony( className = edgeSource; this.edge = edge; this.cycleCount = cycleCount; - changePronenessRank = Math.toIntExact(sourceDisharmonyCount); - edgeTargetChangePronenessRank = Math.toIntExact(targetDisharmonyCount); + edgeSourceDisharmonyCount = Math.toIntExact(sourceDisharmonyCount); + edgeTargetDisharmonyCount = Math.toIntExact(targetDisharmonyCount); effortRank = weight; this.sourceNodeShouldBeRemoved = sourceNodeShouldBeRemoved ? 1 : 0; this.targetNodeShouldBeRemoved = targetNodeShouldBeRemoved ? 1 : 0; diff --git a/cost-benefit-calculator/src/test/java/org/hjug/cbc/CostBenefitCalculatorTest.java b/cost-benefit-calculator/src/test/java/org/hjug/cbc/CostBenefitCalculatorTest.java index 7609254a..a73dabdf 100644 --- a/cost-benefit-calculator/src/test/java/org/hjug/cbc/CostBenefitCalculatorTest.java +++ b/cost-benefit-calculator/src/test/java/org/hjug/cbc/CostBenefitCalculatorTest.java @@ -105,7 +105,7 @@ void testCostBenefitCalculation() throws IOException, GitAPIException, Interrupt } @Test - void calculateSourceNodeCostBenefitValues_filtersMissingLogInfoAndAssignsPriority() throws Exception { + void calculateRelationshipCostBenefitValues_filtersMissingLogInfoAndAssignsPriority() throws Exception { writeFile(hudsonPath + "Dummy.java", "public class Dummy {}"); @@ -156,7 +156,7 @@ void calculateSourceNodeCostBenefitValues_filtersMissingLogInfoAndAssignsPriorit CodebaseGraphDTO dto = mock(CodebaseGraphDTO.class); when(dto.getClassDisharmonyCountForClass(any())).thenReturn(0L); - List disharmonies = costBenefitCalculator.calculateSourceNodeCostBenefitValues( + List disharmonies = costBenefitCalculator.calculateRelationshipCostBenefitValues( classGraph, edgeToRemoveCycleCounts, dto, vertexesToRemove); Assertions.assertEquals(2, disharmonies.size()); @@ -176,12 +176,12 @@ void calculateSourceNodeCostBenefitValues_filtersMissingLogInfoAndAssignsPriorit Assertions.assertEquals(0, classC.getSourceNodeShouldBeRemoved()); Assertions.assertEquals(1, classC.getTargetNodeShouldBeRemoved()); Assertions.assertEquals(2, classC.getPriority().intValue()); - Assertions.assertEquals(0, classC.getChangePronenessRank()); + Assertions.assertEquals(0, classC.getEdgeSourceDisharmonyCount()); } } @Test - void calculateSourceNodeCostBenefitValues_prefersHigherChangePronenessRank() throws Exception { + void calculateRelationshipCostBenefitValues_prefersHigherChangePronenessRank() throws Exception { writeFile(faceletsPath + "Placeholder.java", "public class Placeholder {}"); @@ -228,14 +228,14 @@ void calculateSourceNodeCostBenefitValues_prefersHigherChangePronenessRank() thr CodebaseGraphDTO dto = mock(CodebaseGraphDTO.class); when(dto.getClassDisharmonyCountForClass(any())).thenReturn(0L).thenReturn(1L); - List disharmonies = costBenefitCalculator.calculateSourceNodeCostBenefitValues( + List disharmonies = costBenefitCalculator.calculateRelationshipCostBenefitValues( classGraph, edgeToRemoveCycleCounts, dto, vertexesToRemove); Assertions.assertEquals(2, disharmonies.size()); - Assertions.assertEquals(0, disharmonies.get(0).getChangePronenessRank()); + Assertions.assertEquals(0, disharmonies.get(0).getEdgeSourceDisharmonyCount()); Assertions.assertEquals(1, disharmonies.get(0).getPriority().intValue()); Assertions.assertEquals(2, disharmonies.get(1).getPriority().intValue()); - Assertions.assertEquals(1, disharmonies.get(1).getChangePronenessRank()); + Assertions.assertEquals(1, disharmonies.get(1).getEdgeSourceDisharmonyCount()); } } @@ -297,7 +297,7 @@ void sortEdgesThatNeedToBeRemoved_sortsByMultipleCriteria() { + disharmony.getEffortRank() + " " + disharmony.getSourceNodeShouldBeRemoved() + " " + disharmony.getTargetNodeShouldBeRemoved() + " " - + disharmony.getChangePronenessRank()); + + disharmony.getEdgeSourceDisharmonyCount()); } RankedDisharmony orderedDisharmony0 = disharmonies.get(0); @@ -306,7 +306,7 @@ void sortEdgesThatNeedToBeRemoved_sortsByMultipleCriteria() { Assertions.assertEquals(1, orderedDisharmony0.getEffortRank().intValue()); Assertions.assertEquals(0, orderedDisharmony0.getSourceNodeShouldBeRemoved()); Assertions.assertEquals(0, orderedDisharmony0.getTargetNodeShouldBeRemoved()); - Assertions.assertEquals(0, orderedDisharmony0.getChangePronenessRank()); + Assertions.assertEquals(0, orderedDisharmony0.getEdgeSourceDisharmonyCount()); RankedDisharmony orderedDisharmony1 = disharmonies.get(1); Assertions.assertEquals("Class2", orderedDisharmony1.getClassName()); @@ -314,7 +314,7 @@ void sortEdgesThatNeedToBeRemoved_sortsByMultipleCriteria() { Assertions.assertEquals(1, orderedDisharmony1.getEffortRank().intValue()); Assertions.assertEquals(1, orderedDisharmony1.getSourceNodeShouldBeRemoved()); Assertions.assertEquals(0, orderedDisharmony1.getTargetNodeShouldBeRemoved()); - Assertions.assertEquals(1, orderedDisharmony1.getChangePronenessRank()); + Assertions.assertEquals(1, orderedDisharmony1.getEdgeSourceDisharmonyCount()); RankedDisharmony orderedDisharmony2 = disharmonies.get(2); Assertions.assertEquals("Class3", orderedDisharmony2.getClassName()); @@ -322,7 +322,7 @@ void sortEdgesThatNeedToBeRemoved_sortsByMultipleCriteria() { Assertions.assertEquals(1, orderedDisharmony2.getEffortRank().intValue()); Assertions.assertEquals(0, orderedDisharmony2.getSourceNodeShouldBeRemoved()); Assertions.assertEquals(1, orderedDisharmony2.getTargetNodeShouldBeRemoved()); - Assertions.assertEquals(2, orderedDisharmony2.getChangePronenessRank()); + Assertions.assertEquals(2, orderedDisharmony2.getEdgeSourceDisharmonyCount()); RankedDisharmony orderedDisharmony3 = disharmonies.get(3); Assertions.assertEquals("Class5", orderedDisharmony3.getClassName()); @@ -330,7 +330,7 @@ void sortEdgesThatNeedToBeRemoved_sortsByMultipleCriteria() { Assertions.assertEquals(1, orderedDisharmony3.getEffortRank().intValue()); Assertions.assertEquals(0, orderedDisharmony3.getSourceNodeShouldBeRemoved()); Assertions.assertEquals(0, orderedDisharmony3.getTargetNodeShouldBeRemoved()); - Assertions.assertEquals(5, orderedDisharmony3.getChangePronenessRank()); + Assertions.assertEquals(5, orderedDisharmony3.getEdgeSourceDisharmonyCount()); RankedDisharmony orderedDisharmony4 = disharmonies.get(4); Assertions.assertEquals("Class4", orderedDisharmony4.getClassName()); @@ -338,7 +338,7 @@ void sortEdgesThatNeedToBeRemoved_sortsByMultipleCriteria() { Assertions.assertEquals(3, orderedDisharmony4.getCycleCount().intValue()); Assertions.assertEquals(0, orderedDisharmony4.getSourceNodeShouldBeRemoved()); Assertions.assertEquals(0, orderedDisharmony4.getTargetNodeShouldBeRemoved()); - Assertions.assertEquals(6, orderedDisharmony4.getChangePronenessRank()); + Assertions.assertEquals(6, orderedDisharmony4.getEdgeSourceDisharmonyCount()); } private void writeFile(String name, String content) throws IOException { diff --git a/graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalComputer.java b/graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalComputer.java new file mode 100644 index 00000000..5395efff --- /dev/null +++ b/graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalComputer.java @@ -0,0 +1,60 @@ +package org.hjug.feedback; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import lombok.extern.slf4j.Slf4j; +import org.hjug.dsm.CircularReferenceChecker; +import org.hjug.feedback.arc.pageRank.PageRankFAS; +import org.hjug.feedback.vertex.kernelized.DirectedFeedbackVertexSetResult; +import org.hjug.feedback.vertex.kernelized.DirectedFeedbackVertexSetSolver; +import org.hjug.feedback.vertex.kernelized.EnhancedParameterComputer; +import org.jgrapht.Graph; +import org.jgrapht.graph.AsSubgraph; +import org.jgrapht.graph.DefaultWeightedEdge; + +@Slf4j +public class CycleRemovalComputer { + + public CycleRemovalResult computeCycleRemovalInformation(Graph graph) { + Map> cycles = + new CircularReferenceChecker().getCycles(graph); + Map edgeCycleCounts = new HashMap<>(); + Set vertexesToRemove = new HashSet<>(); + Set edgesToRemove = new HashSet<>(); + + // Skip vertex and edge removal analysis if there are no cycles + if (!cycles.isEmpty()) { + // Identify vertexes to remove + log.info("Identifying vertexes to remove"); + EnhancedParameterComputer enhancedParameterComputer = + new EnhancedParameterComputer<>(new SuperTypeToken<>() {}); + EnhancedParameterComputer.EnhancedParameters parameters = + enhancedParameterComputer.computeOptimalParameters(graph, 4); + DirectedFeedbackVertexSetSolver vertexSolver = + new DirectedFeedbackVertexSetSolver<>( + graph, parameters.getModulator(), null, parameters.getEta(), new SuperTypeToken<>() {}); + DirectedFeedbackVertexSetResult vertexSetResult = vertexSolver.solve(parameters.getK()); + vertexesToRemove.addAll(vertexSetResult.getFeedbackVertices()); + + // Identify edges to remove + log.info("Identifying edges to remove"); + PageRankFAS pageRankFAS = new PageRankFAS<>(graph, new SuperTypeToken<>() {}); + edgesToRemove.addAll(pageRankFAS.computeFeedbackArcSet()); + + // capture the number of cycles each edge to remove is in + for (DefaultWeightedEdge edgeToRemove : edgesToRemove) { + int cycleCount = 0; + for (AsSubgraph cycle : cycles.values()) { + if (cycle.containsEdge(edgeToRemove)) { + cycleCount++; + } + } + edgeCycleCounts.put(edgeToRemove, cycleCount); + } + } + + return new CycleRemovalResult(cycles, edgesToRemove, vertexesToRemove, edgeCycleCounts); + } +} diff --git a/graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalResult.java b/graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalResult.java new file mode 100644 index 00000000..9e240b98 --- /dev/null +++ b/graph-algorithms/src/main/java/org/hjug/feedback/CycleRemovalResult.java @@ -0,0 +1,15 @@ +package org.hjug.feedback; + +import java.util.Map; +import java.util.Set; +import lombok.Data; +import org.jgrapht.graph.AsSubgraph; +import org.jgrapht.graph.DefaultWeightedEdge; + +@Data +public class CycleRemovalResult { + private final Map> cycles; + private final Set edgesToRemove; + private final Set vertexesToRemove; + private final Map edgeCycleCounts; +} diff --git a/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java index 87d923b1..75c040ef 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java @@ -561,13 +561,13 @@ String buildClassGraphDot( } // render vertices - renderVertices(classGraph, repoUrl, codebaseGraphDTO, vertexesToRender, dot); + renderClassVertices(classGraph, repoUrl, codebaseGraphDTO, vertexesToRender, dot); dot.append("}`;"); return dot.toString(); } - private void renderVertices( + private void renderClassVertices( Graph classGraph, String repoUrl, CodebaseGraphDTO codebaseGraphDTO, @@ -591,7 +591,7 @@ private void renderVertices( dot.append(" label=\"").append(className.replace("$", "\\$")).append("\""); } - if (vertexesToRemove.contains(vertex)) { + if (classesToRemove.contains(vertex)) { dot.append(" color=red style=filled"); } @@ -645,7 +645,7 @@ private void renderEdge( dot.append(edgeWeight); dot.append("\""); - if (edgesToRemove.contains(edge)) { + if (classRelationshipsToRemove.contains(edge)) { dot.append(" color = \"red\""); } @@ -688,7 +688,7 @@ String buildCycleDot( // render vertices Set vertexSet = cycle.getVertexSet(); - renderVertices(classGraph, repoUrl, codebaseGraphDTO, vertexSet, dot); + renderClassVertices(classGraph, repoUrl, codebaseGraphDTO, vertexSet, dot); dot.append("}`;"); return dot.toString(); diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java index c0537b65..41a6b543 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java @@ -16,12 +16,8 @@ import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import org.hjug.cbc.*; -import org.hjug.dsm.CircularReferenceChecker; -import org.hjug.feedback.SuperTypeToken; -import org.hjug.feedback.arc.pageRank.PageRankFAS; -import org.hjug.feedback.vertex.kernelized.DirectedFeedbackVertexSetResult; -import org.hjug.feedback.vertex.kernelized.DirectedFeedbackVertexSetSolver; -import org.hjug.feedback.vertex.kernelized.EnhancedParameterComputer; +import org.hjug.feedback.CycleRemovalComputer; +import org.hjug.feedback.CycleRemovalResult; import org.hjug.git.GitLogReader; import org.hjug.graphbuilder.CodebaseGraphDTO; import org.hjug.graphbuilder.metrics.DisharmonyMetric; @@ -50,9 +46,13 @@ public class SimpleHtmlReport { public final String[] classCycleTableHeadings = {"Classes", "Relationships"}; Graph classGraph; + Graph packageGraph; Map> classCycles; - Set vertexesToRemove = Set.of(); // initialize for unit tests - Set edgesToRemove = Set.of(); + Map> packageCycles; + Set classesToRemove = Set.of(); // initialize for unit tests + Set packagesToRemove = Set.of(); // initialize for unit tests + Set classRelationshipsToRemove = Set.of(); + Set packageRelationshipsToRemove = Set.of(); DateTimeFormatter formatter = DateTimeFormatter.ofLocalizedDateTime(FormatStyle.SHORT) .withLocale(Locale.getDefault()) @@ -132,16 +132,14 @@ public StringBuilder generateReport( stringBuilder.append(printBreadcrumbs()); stringBuilder.append(printProjectHeader(projectName, projectVersion)); - GitLogReader gitLogReader = new GitLogReader(); String projectBaseDir; Optional optionalGitDir; - if (baseDir != null) { projectBaseDir = baseDir.getPath(); - optionalGitDir = Optional.ofNullable(gitLogReader.getGitDir(baseDir)); + optionalGitDir = Optional.ofNullable(GitLogReader.getGitDir(baseDir)); } else { projectBaseDir = Paths.get("").toAbsolutePath().toString(); - optionalGitDir = Optional.ofNullable(gitLogReader.getGitDir(new File(projectBaseDir))); + optionalGitDir = Optional.ofNullable(GitLogReader.getGitDir(new File(projectBaseDir))); } File gitDir; @@ -174,56 +172,41 @@ public StringBuilder generateReport( CycleRanker cycleRanker = new CycleRanker(projectBaseDir); List rankedClassCycles = List.of(); + // List rankedPackageCycles = List.of(); CodebaseGraphDTO codebaseGraphDTO; if (analyzeCycles) { log.info("Analyzing Cycles"); cycleRanker.generateClassReferencesGraph(excludeTests, testSourceDirectory); codebaseGraphDTO = cycleRanker.getCodebaseGraphDTO(); - rankedClassCycles = cycleRanker.performCycleAnalysis(codebaseGraphDTO.getClassReferencesGraph()); + rankedClassCycles = cycleRanker.rankCycles(codebaseGraphDTO.getClassReferencesGraph()); + // rankedPackageCycles = cycleRanker.rankCycles(codebaseGraphDTO.getPackageReferencesGraph()); } else { codebaseGraphDTO = cycleRanker.generateClassReferencesGraph(excludeTests, testSourceDirectory); } classGraph = codebaseGraphDTO.getClassReferencesGraph(); - classCycles = new CircularReferenceChecker().getCycles(classGraph); - Map edgeCycleCounts = new HashMap<>(); - - // Skip vertex and edge removal analysis if there are no cycles - if (!classCycles.isEmpty()) { - // Identify vertexes to remove - log.info("Identifying vertexes to remove"); - EnhancedParameterComputer enhancedParameterComputer = - new EnhancedParameterComputer<>(new SuperTypeToken<>() {}); - EnhancedParameterComputer.EnhancedParameters parameters = - enhancedParameterComputer.computeOptimalParameters(classGraph, 4); - DirectedFeedbackVertexSetSolver vertexSolver = - new DirectedFeedbackVertexSetSolver<>( - classGraph, - parameters.getModulator(), - null, - parameters.getEta(), - new SuperTypeToken<>() {}); - DirectedFeedbackVertexSetResult vertexSetResult = vertexSolver.solve(parameters.getK()); - vertexesToRemove = vertexSetResult.getFeedbackVertices(); - - // Identify edges to remove - log.info("Identifying edges to remove"); - PageRankFAS pageRankFAS = - new PageRankFAS<>(classGraph, new SuperTypeToken<>() {}); - edgesToRemove = pageRankFAS.computeFeedbackArcSet(); - - // capture the number of cycles each edge to remove is in - for (DefaultWeightedEdge edgeToRemove : edgesToRemove) { - int cycleCount = 0; - for (AsSubgraph cycle : classCycles.values()) { - if (cycle.containsEdge(edgeToRemove)) { - cycleCount++; - } - } - edgeCycleCounts.put(edgeToRemove, cycleCount); - } + + CycleRemovalComputer cycleRemovalComputer = new CycleRemovalComputer(); + + CycleRemovalResult classCycleRemovalResult = cycleRemovalComputer.computeCycleRemovalInformation(classGraph); + Map classEdgeCycleCounts = classCycleRemovalResult.getEdgeCycleCounts(); + classRelationshipsToRemove = classCycleRemovalResult.getEdgesToRemove(); + classesToRemove = classCycleRemovalResult.getVertexesToRemove(); + classCycles = classCycleRemovalResult.getCycles(); + + packageGraph = codebaseGraphDTO.getPackageReferencesGraph(); + + for (DefaultWeightedEdge defaultWeightedEdge : packageGraph.edgeSet()) { + log.warn(defaultWeightedEdge.toString() + ": " + packageGraph.getEdgeWeight(defaultWeightedEdge)); } + CycleRemovalResult packageCycleRemovalResult = + cycleRemovalComputer.computeCycleRemovalInformation(packageGraph); + Map packageEdgeCycleCounts = packageCycleRemovalResult.getEdgeCycleCounts(); + packageRelationshipsToRemove = packageCycleRemovalResult.getEdgesToRemove(); + packagesToRemove = packageCycleRemovalResult.getVertexesToRemove(); + packageCycles = packageCycleRemovalResult.getCycles(); + // Ordered (type, anchorId, displayTitle, isMethodLevel) for all disharmonies final List disharmonySpecs = List.of( new DisharmonySpec(DisharmonyTypes.GOD_CLASS, "GOD", "God Classes", false), @@ -246,11 +229,14 @@ public StringBuilder generateReport( log.info("Identifying Object Oriented Disharmonies"); - List edgeDisharmonies = List.of(); + List classRelationshipDisharmonies = List.of(); + List packageRelationshipDisharmonies = List.of(); try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir, codebaseGraphDTO.getClassToSourceFilePathMapping())) { - edgeDisharmonies = costBenefitCalculator.calculateSourceNodeCostBenefitValues( - classGraph, edgeCycleCounts, codebaseGraphDTO, vertexesToRemove); + classRelationshipDisharmonies = costBenefitCalculator.calculateRelationshipCostBenefitValues( + classGraph, classEdgeCycleCounts, codebaseGraphDTO, classesToRemove); + packageRelationshipDisharmonies = costBenefitCalculator.calculateRelationshipCostBenefitValues( + packageGraph, packageEdgeCycleCounts, codebaseGraphDTO, packagesToRemove); for (DisharmonySpec spec : disharmonySpecs) { List instances = spec.methodLevel() @@ -273,8 +259,10 @@ public StringBuilder generateReport( // - Edge weight // - Provide guidance on where to move the method if one is in the list to remove - boolean hasAnyDisharmony = - !edgesToRemove.isEmpty() || !rankedClassCycles.isEmpty() || !rankedDisharmoniesByAnchor.isEmpty(); + boolean hasAnyDisharmony = !classRelationshipsToRemove.isEmpty() + || !packageRelationshipsToRemove.isEmpty() + || !rankedClassCycles.isEmpty() + || !rankedDisharmoniesByAnchor.isEmpty(); String repoUrl; try (GitLogReader glr = new GitLogReader(new File(projectBaseDir))) { @@ -305,11 +293,19 @@ public StringBuilder generateReport( stringBuilder.append(renderGithubButtons()); stringBuilder.append("
\n"); - if (!edgeDisharmonies.isEmpty()) { - stringBuilder.append(renderEdgeDisharmonies(edgeDisharmonies, repoUrl, codebaseGraphDTO)); + if (!classRelationshipDisharmonies.isEmpty()) { + stringBuilder.append(renderClassEdgeDisharmonies(classRelationshipDisharmonies, repoUrl, codebaseGraphDTO)); stringBuilder.append("
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n"); } + if (!packageRelationshipDisharmonies.isEmpty()) { + stringBuilder.append( + renderPackageEdgeDisharmonies(packageRelationshipDisharmonies, repoUrl, codebaseGraphDTO)); + stringBuilder.append("
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n"); + } else { + log.info("No Package Relationship Disharmonies found"); + } + for (DisharmonySpec spec : disharmonySpecs) { List rankedForType = rankedDisharmoniesByAnchor.get(spec.anchorId()); if (rankedForType != null && !rankedForType.isEmpty()) { @@ -332,7 +328,7 @@ StringBuilder createMenu( Map> rankedDisharmoniesByAnchor, List rankedCycles) { StringBuilder menu = new StringBuilder(); - if (!edgesToRemove.isEmpty()) { + if (!classRelationshipsToRemove.isEmpty()) { menu.append("
  • Edges To Remove
    • \n"); } @@ -373,19 +369,22 @@ private String renderCycles(List rankedCycles, String repoUrl, Code return stringBuilder.toString(); } - private String renderEdgeDisharmonies( + private String renderClassEdgeDisharmonies( List edgeDisharmonies, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { StringBuilder stringBuilder = new StringBuilder(); stringBuilder.append( - "\n"); + "\n"); stringBuilder.append("

    Refactor Starting with Priority 1

    \n"); stringBuilder.append("
    \n"); - stringBuilder.append("Current Cycle Count: ").append(classCycles.size()).append("
    \n"); + stringBuilder + .append("Current Class Cycle Count: ") + .append(classCycles.size()) + .append("
    \n"); stringBuilder .append("Number of Relationships to Remove: ") - .append(edgesToRemove.size()) + .append(classRelationshipsToRemove.size()) .append("
    \n"); stringBuilder .append("Classes with * should be broken apart") @@ -396,7 +395,7 @@ private String renderEdgeDisharmonies( stringBuilder.append("
    "); stringBuilder.append("\n"); stringBuilder.append("\n\n"); - for (String heading : getEdgeDisharmonyTableHeadings()) { + for (String heading : getClassRelationshipDisharmonyTableHeadings()) { stringBuilder.append("\n"); } stringBuilder.append("\n"); @@ -406,7 +405,7 @@ private String renderEdgeDisharmonies( for (RankedDisharmony edge : edgeDisharmonies) { stringBuilder.append("\n"); - for (String rowData : getEdgeDisharmony(edge, repoUrl, codebaseGraphDTO)) { + for (String rowData : getClassRelationshipDisharmony(edge, repoUrl, codebaseGraphDTO)) { stringBuilder.append(drawTableCell(rowData)); } @@ -420,7 +419,74 @@ private String renderEdgeDisharmonies( return stringBuilder.toString(); } - private String[] getEdgeDisharmonyTableHeadings() { + private String renderPackageEdgeDisharmonies( + List edgeDisharmonies, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + StringBuilder stringBuilder = new StringBuilder(); + + stringBuilder.append( + "\n"); + stringBuilder.append("

    Refactor Starting with Priority 1

    \n"); + stringBuilder.append("
    \n"); + stringBuilder + .append("Current Package Cycle Count: ") + .append(packageCycles.size()) + .append("
    \n"); + + stringBuilder + .append("Number of Relationships to Remove: ") + .append(packageRelationshipsToRemove.size()) + .append("
    \n"); + stringBuilder + .append("Packages with * should be broken apart") + .append("
    \n"); + stringBuilder.append("
    \n"); + + // Content + stringBuilder.append("
    "); + stringBuilder.append("
    ").append(heading).append("
    \n"); + stringBuilder.append("\n\n"); + for (String heading : getPackageRelationshipDisharmonyTableHeadings()) { + stringBuilder.append("\n"); + } + stringBuilder.append("\n"); + + stringBuilder.append("\n"); + + for (RankedDisharmony edge : edgeDisharmonies) { + stringBuilder.append("\n"); + + for (String rowData : getPackageRelationshipDisharmony(edge, repoUrl, codebaseGraphDTO)) { + stringBuilder.append(drawTableCell(rowData)); + } + + stringBuilder.append("\n"); + } + + stringBuilder.append("\n"); + stringBuilder.append("
    ").append(heading).append("
    \n"); + stringBuilder.append("
    \n"); + + return stringBuilder.toString(); + } + + /* + when: com.parentPackage.package + then: com.parentPackage + + when: com.package.ClassA + then: com.package + */ + String getParentNamespace(String fqn) { + // handle no package + if (!fqn.contains(".")) { + return ""; + } + + int lastIndex = fqn.lastIndexOf("."); + return fqn.substring(0, lastIndex); + } + + private String[] getClassRelationshipDisharmonyTableHeadings() { return new String[] { "Relationship", "Priority", @@ -431,14 +497,31 @@ private String[] getEdgeDisharmonyTableHeadings() { }; } - private String[] getEdgeDisharmony(RankedDisharmony edgeInfo, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + private String[] getPackageRelationshipDisharmonyTableHeadings() { + return new String[] { + "Relationship", "Priority", "In Cycles", "Edge
    Weight", + }; + } + + private String[] getClassRelationshipDisharmony( + RankedDisharmony edgeInfo, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { return new String[] { - renderEdge(edgeInfo.getEdge(), repoUrl, codebaseGraphDTO), + renderClassEdge(edgeInfo.getEdge(), repoUrl, codebaseGraphDTO), + String.valueOf(edgeInfo.getPriority()), + String.valueOf(edgeInfo.getCycleCount()), + String.valueOf(edgeInfo.getEffortRank()), + String.valueOf(edgeInfo.getEdgeSourceDisharmonyCount()), + String.valueOf(edgeInfo.getEdgeTargetDisharmonyCount()), + }; + } + + private String[] getPackageRelationshipDisharmony( + RankedDisharmony edgeInfo, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + return new String[] { + renderPackageEdge(edgeInfo.getEdge(), repoUrl, codebaseGraphDTO), String.valueOf(edgeInfo.getPriority()), String.valueOf(edgeInfo.getCycleCount()), String.valueOf(edgeInfo.getEffortRank()), - String.valueOf(edgeInfo.getChangePronenessRank()), - String.valueOf(edgeInfo.getEdgeTargetChangePronenessRank()), }; } @@ -446,10 +529,6 @@ private String renderClassCycleSummary(List rankedCycles) { StringBuilder stringBuilder = new StringBuilder(); stringBuilder.append("\n"); - /*if (rankedCycles.size() > 10) { - stringBuilder.append( - "
    10 largest cycles are shown in the sections below
    \n"); - }*/ stringBuilder.append("

    Class Cycles by the numbers:

    \n"); stringBuilder.append("
    "); @@ -457,7 +536,7 @@ private String renderClassCycleSummary(List rankedCycles) { // Content stringBuilder.append("\n\n"); - for (String heading : getCycleSummaryTableHeadings()) { + for (String heading : getClassCycleSummaryTableHeadings()) { stringBuilder.append("").append(heading).append("\n"); } stringBuilder.append("\n"); @@ -467,19 +546,19 @@ private String renderClassCycleSummary(List rankedCycles) { stringBuilder.append("\n"); StringBuilder edges = new StringBuilder(); - for (DefaultWeightedEdge edge : cycle.getMinCutEdges()) { + for (DefaultWeightedEdge edge : cycle.getEdgeSet()) { - if (edgesToRemove.contains(edge)) { + if (classRelationshipsToRemove.contains(edge)) { stringBuilder.append(""); - edges.append(renderEdge(edge)); + edges.append(renderClassEdge(edge)); stringBuilder.append(""); } else { - edges.append(renderEdge(edge)); + edges.append(renderClassEdge(edge)); } edges.append("
    \n"); } - for (String rowData : getRankedCycleSummaryData(cycle, edges)) { + for (String rowData : getRankedCycleSummaryData(cycle)) { stringBuilder.append(drawTableCell(rowData)); } @@ -492,13 +571,13 @@ private String renderClassCycleSummary(List rankedCycles) { return stringBuilder.toString(); } - private String renderEdge(DefaultWeightedEdge edge) { + private String renderClassEdge(DefaultWeightedEdge edge) { StringBuilder edgesToCut = new StringBuilder(); String[] vertexes = extractVertexes(edge); String startVertex = vertexes[0].trim(); String start; - if (vertexesToRemove.contains(startVertex)) { + if (classesToRemove.contains(startVertex)) { start = "" + getClassName(startVertex) + ""; } else { start = getClassName(startVertex); @@ -506,7 +585,7 @@ private String renderEdge(DefaultWeightedEdge edge) { String endVertex = vertexes[1].trim(); String end; - if (vertexesToRemove.contains(endVertex)) { + if (classesToRemove.contains(endVertex)) { end = "" + getClassName(endVertex) + ""; } else { end = getClassName(endVertex); @@ -518,13 +597,13 @@ private String renderEdge(DefaultWeightedEdge edge) { .toString(); } - private String renderEdge(DefaultWeightedEdge edge, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + private String renderClassEdge(DefaultWeightedEdge edge, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { StringBuilder edgesToCut = new StringBuilder(); String[] vertexes = extractVertexes(edge); String startVertex = vertexes[0].trim(); String start; - if (vertexesToRemove.contains(startVertex)) { + if (classesToRemove.contains(startVertex)) { start = hyperlinkClass(startVertex, repoUrl, codebaseGraphDTO) + "*"; } else { start = hyperlinkClass(startVertex, repoUrl, codebaseGraphDTO); @@ -532,7 +611,7 @@ private String renderEdge(DefaultWeightedEdge edge, String repoUrl, CodebaseGrap String endVertex = vertexes[1].trim(); String end; - if (vertexesToRemove.contains(endVertex)) { + if (classesToRemove.contains(endVertex)) { end = hyperlinkClass(endVertex, repoUrl, codebaseGraphDTO) + "*"; } else { end = hyperlinkClass(endVertex, repoUrl, codebaseGraphDTO); @@ -544,6 +623,32 @@ private String renderEdge(DefaultWeightedEdge edge, String repoUrl, CodebaseGrap .toString(); } + private String renderPackageEdge(DefaultWeightedEdge edge, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + StringBuilder edgesToCut = new StringBuilder(); + String[] vertexes = extractVertexes(edge); + + String startVertex = vertexes[0].trim(); + String start; + if (packagesToRemove.contains(startVertex)) { + start = startVertex + "*"; + } else { + start = startVertex; + } + + String endVertex = vertexes[1].trim(); + String end; + if (packagesToRemove.contains(endVertex)) { + end = endVertex + "*"; + } else { + end = endVertex; + } + + // → is HTML "Right Arrow" code + return edgesToCut + .append(start + " → " + end + " : " + (int) packageGraph.getEdgeWeight(edge)) + .toString(); + } + String hyperlinkClass(String className, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { StringBuilder sb = new StringBuilder(); String path = codebaseGraphDTO.getClassToSourceFilePathMapping().get(className); @@ -551,13 +656,13 @@ String hyperlinkClass(String className, String repoUrl, CodebaseGraphDTO codebas .toString(); } - private String[] getCycleSummaryTableHeadings() { - return new String[] {"Cycle Name", "Priority", "Class Count", "Relationship Count" /*, "Minimum Cuts"*/}; + private String[] getClassCycleSummaryTableHeadings() { + return new String[] {"Cycle Name", "Priority", "Class Count", "Relationship Count"}; } - private String[] getRankedCycleSummaryData(RankedCycle rankedCycle, StringBuilder edgesToCut) { + private String[] getRankedCycleSummaryData(RankedCycle rankedCycle) { return new String[] { - // "Cycle Name", "Priority", "Class Count", "Relationship Count", "Min Cuts" + // "Cycle Name", "Priority", "Class Count", "Relationship Count" getClassName(rankedCycle.getCycleName()), rankedCycle.getPriority().toString(), String.valueOf(rankedCycle.getCycleNodes().size()), @@ -606,7 +711,7 @@ private String renderSingleCycle(RankedCycle cycle, String repoUrl, CodebaseGrap for (String vertex : cycle.getVertexSet()) { stringBuilder.append(""); String className; - if (vertexesToRemove.contains(vertex)) { + if (classesToRemove.contains(vertex)) { className = hyperlinkClass(vertex, repoUrl, codebaseGraphDTO) + "*"; } else { className = hyperlinkClass(vertex, repoUrl, codebaseGraphDTO); @@ -617,12 +722,12 @@ private String renderSingleCycle(RankedCycle cycle, String repoUrl, CodebaseGrap for (DefaultWeightedEdge edge : cycle.getEdgeSet()) { if (edge.toString().startsWith("(" + vertex + " :")) { - if (edgesToRemove.contains(edge)) { + if (classRelationshipsToRemove.contains(edge)) { edges.append(""); - edges.append(renderEdge(edge)); + edges.append(renderClassEdge(edge)); edges.append(""); } else { - edges.append(renderEdge(edge)); + edges.append(renderClassEdge(edge)); } edges.append("
    \n"); @@ -716,7 +821,14 @@ public String printProjectFooter() { } String renderGithubButtons() { - return ""; // empty on purpose + return "
    \n" + "Show RefactorFirst some ❤️\n" + + "
    \n" + + "Star\n" + + "Fork\n" + + "Watch\n" + + "Issue\n" + + "Sponsor\n" + + "
    "; } String getOutputName() { diff --git a/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java b/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java index 585ff578..bc2cf60b 100644 --- a/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java +++ b/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java @@ -51,8 +51,7 @@ void buildCycleDot() { String cycleName = "Test"; List cycleNodes = new ArrayList<>(); - RankedCycle rankedCycle = - new RankedCycle(cycleName, 0, classGraph.vertexSet(), classGraph.edgeSet(), 0, null, cycleNodes); + RankedCycle rankedCycle = new RankedCycle(cycleName, classGraph.vertexSet(), classGraph.edgeSet(), cycleNodes); HtmlReport htmlReport = new HtmlReport(); CodebaseGraphDTO dto = mock(CodebaseGraphDTO.class); From cb8ee2df47ed229f4a9356227fc8fe17a539c36b Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Wed, 17 Jun 2026 12:45:09 -0500 Subject: [PATCH 3/9] #185 Listing class relationship(s) for each package relationship that needs to be removed --- .../hjug/graphbuilder/CodebaseGraphDTO.java | 4 ++ .../graphbuilder/DependencyCollector.java | 4 +- .../GraphDependencyCollector.java | 46 +++++++++++++++---- .../hjug/graphbuilder/JavaGraphBuilder.java | 21 +++++++++ .../metrics/GraphMetricsCollector.java | 4 +- .../hjug/cbc/DisharmonyChurnRankingTest.java | 1 + .../hjug/cbc/DisharmonyExtractionTest.java | 2 + .../report/SimpleHtmlReport.java | 32 +++++-------- 8 files changed, 83 insertions(+), 31 deletions(-) diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/CodebaseGraphDTO.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/CodebaseGraphDTO.java index f705ea04..8519332f 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/CodebaseGraphDTO.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/CodebaseGraphDTO.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -19,6 +20,7 @@ public class CodebaseGraphDTO { private final Graph classReferencesGraph; private final Graph packageReferencesGraph; + private final Map> classRelationshipsInPackageRelationship; // used for looking up files where classes reside private final Map classToSourceFilePathMapping; @@ -29,11 +31,13 @@ public class CodebaseGraphDTO { public CodebaseGraphDTO( Graph classReferencesGraph, Graph packageReferencesGraph, + Map> classRelationshipsInPackageRelationship, Map classToSourceFilePathMapping, List classDisharmonies, List methodDisharmonies) { this.classReferencesGraph = classReferencesGraph; this.packageReferencesGraph = packageReferencesGraph; + this.classRelationshipsInPackageRelationship = classRelationshipsInPackageRelationship; this.classToSourceFilePathMapping = classToSourceFilePathMapping; this.classDisharmonies = classDisharmonies; this.methodDisharmonies = methodDisharmonies; diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/DependencyCollector.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/DependencyCollector.java index 51987fa6..8a48a000 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/DependencyCollector.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/DependencyCollector.java @@ -1,5 +1,7 @@ package org.hjug.graphbuilder; +import org.jgrapht.graph.DefaultWeightedEdge; + // TODO: Revisit - I don't think this is really needed public interface DependencyCollector { @@ -17,7 +19,7 @@ public interface DependencyCollector { * @param fromPackageName The package that depends on another * @param toPackageName The package being depended upon */ - void addPackageDependency(String fromPackageName, String toPackageName); + DefaultWeightedEdge addPackageDependency(String fromPackageName, String toPackageName); /** * Records the source file location for a class diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java index 73523f96..8a086249 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java @@ -1,11 +1,15 @@ package org.hjug.graphbuilder; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import lombok.Getter; +import lombok.extern.slf4j.Slf4j; import org.jgrapht.Graph; import org.jgrapht.graph.DefaultWeightedEdge; +@Slf4j public class GraphDependencyCollector implements DependencyCollector { @Getter @@ -17,6 +21,10 @@ public class GraphDependencyCollector implements DependencyCollector { @Getter private final Set packagesInCodebase = new HashSet<>(); + @Getter + private final Map> classRelationshipsInPackageRelationship = + new HashMap<>(); + public GraphDependencyCollector( Graph classReferencesGraph, Graph packageReferencesGraph) { @@ -33,31 +41,51 @@ public void addClassDependency(String fromClassFqn, String toClassFqn) { classReferencesGraph.addVertex(fromClassFqn); classReferencesGraph.addVertex(toClassFqn); + DefaultWeightedEdge classRelationship; if (!classReferencesGraph.containsEdge(fromClassFqn, toClassFqn)) { - classReferencesGraph.addEdge(fromClassFqn, toClassFqn); + classRelationship = classReferencesGraph.addEdge(fromClassFqn, toClassFqn); } else { - DefaultWeightedEdge edge = classReferencesGraph.getEdge(fromClassFqn, toClassFqn); - classReferencesGraph.setEdgeWeight(edge, classReferencesGraph.getEdgeWeight(edge) + 1); + classRelationship = classReferencesGraph.getEdge(fromClassFqn, toClassFqn); + classReferencesGraph.setEdgeWeight( + classRelationship, classReferencesGraph.getEdgeWeight(classRelationship) + 1); } - addPackageDependency(getPackageFromFqn(fromClassFqn), getPackageFromFqn(toClassFqn)); + DefaultWeightedEdge packageEdge = addPackageDependency(fromClassFqn, toClassFqn); + + if (packageEdge != null) { + if (!classRelationshipsInPackageRelationship.containsKey(packageEdge)) { + classRelationshipsInPackageRelationship.put(packageEdge, new HashSet<>()); + } + + Set packageRelationship = classRelationshipsInPackageRelationship.get(packageEdge); + packageRelationship.remove(classRelationship); + packageRelationship.add(classRelationship); + } } @Override - public void addPackageDependency(String fromPackageName, String toPackageName) { + public DefaultWeightedEdge addPackageDependency(String fromClassFqn, String toClassFqn) { + String fromPackageName = getPackageFromFqn(fromClassFqn); + String toPackageName = getPackageFromFqn(toClassFqn); + if (fromPackageName.equals(toPackageName)) { - return; + return null; } packageReferencesGraph.addVertex(fromPackageName); packageReferencesGraph.addVertex(toPackageName); + DefaultWeightedEdge packageEdge; if (!packageReferencesGraph.containsEdge(fromPackageName, toPackageName)) { - packageReferencesGraph.addEdge(fromPackageName, toPackageName); + packageEdge = packageReferencesGraph.addEdge(fromPackageName, toPackageName); } else { - DefaultWeightedEdge edge = packageReferencesGraph.getEdge(fromPackageName, toPackageName); - packageReferencesGraph.setEdgeWeight(edge, packageReferencesGraph.getEdgeWeight(edge) + 1); + packageEdge = packageReferencesGraph.getEdge(fromPackageName, toPackageName); + packageReferencesGraph.setEdgeWeight(packageEdge, packageReferencesGraph.getEdgeWeight(packageEdge) + 1); } + + log.warn("Package dependency: {} -> {}", fromClassFqn, toClassFqn); + + return packageEdge; } protected String getPackageFromFqn(String fqn) { diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/JavaGraphBuilder.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/JavaGraphBuilder.java index af5df094..385858f7 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/JavaGraphBuilder.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/JavaGraphBuilder.java @@ -99,6 +99,12 @@ private CodebaseGraphDTO processWithOpenRewrite(String repositoryPath, GraphBuil } removeClassesNotInCodebase(dependencyCollector.getPackagesInCodebase(), classReferencesGraph); + removePackagesNotInCodebase(dependencyCollector.getPackagesInCodebase(), packageReferencesGraph); + // remove class relationships that are not in the codebase that are in the package -> class relationship mapping + dependencyCollector + .getClassRelationshipsInPackageRelationship() + .keySet() + .retainAll(packageReferencesGraph.edgeSet()); metricsCollector.finalizeMetrics(); DisharmonyDetector detector = new DisharmonyDetector(); @@ -107,6 +113,7 @@ private CodebaseGraphDTO processWithOpenRewrite(String repositoryPath, GraphBuil return new CodebaseGraphDTO( classReferencesGraph, packageReferencesGraph, + dependencyCollector.getClassRelationshipsInPackageRelationship(), javaVisitor.getClassToSourceFilePathMapping(), // hudson.model.FilePath -> // file:///C:/Code/RefactorFirst/cost-benefit-calculator/hudson/model/FilePath.java getClassDisharmonies(detector, metrics), @@ -151,6 +158,20 @@ void removeClassesNotInCodebase( classReferencesGraph.removeAllVertices(classesToRemove); } + void removePackagesNotInCodebase( + Set packagesInCodebase, Graph packageReferencesGraph) { + + // collect nodes to remove + Set packagesToRemove = new HashSet<>(); + for (String aPackage : packageReferencesGraph.vertexSet()) { + if (!packagesInCodebase.contains(aPackage)) { + packagesToRemove.add(aPackage); + } + } + + packageReferencesGraph.removeAllVertices(packagesToRemove); + } + String getPackage(String fqn) { // handle no package if (!fqn.contains(".")) { diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/GraphMetricsCollector.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/GraphMetricsCollector.java index 1ff4eefa..45cd7546 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/GraphMetricsCollector.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/GraphMetricsCollector.java @@ -50,7 +50,7 @@ public void addClassDependency(String fromClass, String toClass) { } @Override - public void addPackageDependency(String fromPackage, String toPackage) { + public DefaultWeightedEdge addPackageDependency(String fromPackage, String toPackage) { if (!packageGraph.containsVertex(fromPackage)) { packageGraph.addVertex(fromPackage); } @@ -68,6 +68,8 @@ public void addPackageDependency(String fromPackage, String toPackage) { double weight = packageGraph.getEdgeWeight(edge); packageGraph.setEdgeWeight(edge, weight + 1.0); } + + return edge; } @Override diff --git a/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyChurnRankingTest.java b/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyChurnRankingTest.java index 9e0ffa90..251ab000 100644 --- a/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyChurnRankingTest.java +++ b/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyChurnRankingTest.java @@ -252,6 +252,7 @@ private CodebaseGraphDTO buildDtoWithMethodDisharmony( return new CodebaseGraphDTO( new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class), new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class), + new HashMap<>(), dtoFileMap, List.of(), List.of(d)); diff --git a/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyExtractionTest.java b/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyExtractionTest.java index 6482efb9..a8c09ba9 100644 --- a/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyExtractionTest.java +++ b/cost-benefit-calculator/src/test/java/org/hjug/cbc/DisharmonyExtractionTest.java @@ -172,6 +172,7 @@ private CodebaseGraphDTO buildDtoWithBrainClass() { return new CodebaseGraphDTO( new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class), new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class), + new HashMap<>(), fileMap, List.of(d), List.of()); @@ -210,6 +211,7 @@ private CodebaseGraphDTO buildDtoWithBrainMethod() { return new CodebaseGraphDTO( new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class), new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class), + new HashMap<>(), fileMap, List.of(), List.of(d)); diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java index 41a6b543..8eb72cdc 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java @@ -383,7 +383,7 @@ private String renderClassEdgeDisharmonies( .append("
    \n"); stringBuilder - .append("Number of Relationships to Remove: ") + .append("Number of Class Relationships to Remove: ") .append(classRelationshipsToRemove.size()) .append("
    \n"); stringBuilder @@ -433,7 +433,7 @@ private String renderPackageEdgeDisharmonies( .append("
    \n"); stringBuilder - .append("Number of Relationships to Remove: ") + .append("Number of Package Relationships to Remove: ") .append(packageRelationshipsToRemove.size()) .append("
    \n"); stringBuilder @@ -469,23 +469,6 @@ private String renderPackageEdgeDisharmonies( return stringBuilder.toString(); } - /* - when: com.parentPackage.package - then: com.parentPackage - - when: com.package.ClassA - then: com.package - */ - String getParentNamespace(String fqn) { - // handle no package - if (!fqn.contains(".")) { - return ""; - } - - int lastIndex = fqn.lastIndexOf("."); - return fqn.substring(0, lastIndex); - } - private String[] getClassRelationshipDisharmonyTableHeadings() { return new String[] { "Relationship", @@ -499,7 +482,7 @@ private String[] getClassRelationshipDisharmonyTableHeadings() { private String[] getPackageRelationshipDisharmonyTableHeadings() { return new String[] { - "Relationship", "Priority", "In Cycles", "Edge
    Weight", + "Package Relationship", "Priority", "In Cycles", "Edge
    Weight", "Class Relationships", }; } @@ -517,11 +500,20 @@ private String[] getClassRelationshipDisharmony( private String[] getPackageRelationshipDisharmony( RankedDisharmony edgeInfo, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + + Set classRelationshipsInPackageRelationship = + codebaseGraphDTO.getClassRelationshipsInPackageRelationship().get(edgeInfo.getEdge()); + Set classEdges = new HashSet<>(); + for (DefaultWeightedEdge defaultWeightedEdge : classRelationshipsInPackageRelationship) { + classEdges.add(renderClassEdge(defaultWeightedEdge, repoUrl, codebaseGraphDTO)); + } + return new String[] { renderPackageEdge(edgeInfo.getEdge(), repoUrl, codebaseGraphDTO), String.valueOf(edgeInfo.getPriority()), String.valueOf(edgeInfo.getCycleCount()), String.valueOf(edgeInfo.getEffortRank()), + String.join("
    ", classEdges), }; } From 216afa21722a71eb0ed5781f557cfe0f8e88c26e Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Thu, 18 Jun 2026 08:02:18 -0500 Subject: [PATCH 4/9] #185 Generating package map --- .../GraphDependencyCollector.java | 2 - .../hjug/refactorfirst/report/HtmlReport.java | 137 +++++++++++++++++- .../report/SimpleHtmlReport.java | 12 +- .../refactorfirst/report/HtmlReportTest.java | 4 +- 4 files changed, 142 insertions(+), 13 deletions(-) diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java index 8a086249..8141c073 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/GraphDependencyCollector.java @@ -83,8 +83,6 @@ public DefaultWeightedEdge addPackageDependency(String fromClassFqn, String toCl packageReferencesGraph.setEdgeWeight(packageEdge, packageReferencesGraph.getEdgeWeight(packageEdge) + 1); } - log.warn("Package dependency: {} -> {}", fromClassFqn, toClassFqn); - return packageEdge; } diff --git a/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java index 75c040ef..d8a2cb7d 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java @@ -420,6 +420,7 @@ StringBuilder createMenu( List rankedCycles) { StringBuilder stringBuilder = new StringBuilder(); stringBuilder.append("
  • Class Map
    • \n"); + stringBuilder.append("
  • Package Map
    • \n"); stringBuilder.append(super.createMenu(disharmonySpecs, rankedDisharmoniesByAnchor, rankedCycles)); return stringBuilder; } @@ -477,6 +478,7 @@ public String renderClassGraphVisuals(String repoUrl, CodebaseGraphDTO codebaseG String classGraphName = "classGraph"; StringBuilder stringBuilder = new StringBuilder(); + stringBuilder.append("

    Class Map

    "); stringBuilder.append(generateGraphButtons(classGraphName, dot)); stringBuilder.append( @@ -498,7 +500,6 @@ public String renderClassGraphVisuals(String repoUrl, CodebaseGraphDTO codebaseG private StringBuilder generateGraphButtons(String graphName, String dot) { StringBuilder stringBuilder = new StringBuilder(); - stringBuilder.append("

    Class Map

    "); stringBuilder.append("\n"); @@ -549,7 +550,7 @@ String buildClassGraphDot( dot.append("`strict digraph G {\n"); for (DefaultWeightedEdge edge : classGraph.edgeSet()) { - renderEdge(classGraph, edge, dot); + renderClassGraphEdge(classGraph, edge, dot); } // capture only classes that have a relationship with one or more other classes @@ -605,7 +606,7 @@ String hyperlinkClassForDot(String fqClassName, String repoUrl, CodebaseGraphDTO return sb.append("URL=\"" + repoUrl + path + "\" target=\"_blank\"").toString(); } - private void renderEdge( + private void renderClassGraphEdge( Graph classGraph, DefaultWeightedEdge edge, StringBuilder dot) { // render edge String[] vertexes = extractVertexes(edge); @@ -653,12 +654,13 @@ private void renderEdge( } @Override - public String renderCycleVisuals(RankedCycle cycle, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { - String dot = buildCycleDot(classGraph, cycle, repoUrl, codebaseGraphDTO); + public String renderClassCycleVisuals(RankedCycle cycle, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + String dot = buildClassCycleDot(classGraph, cycle, repoUrl, codebaseGraphDTO); String cycleName = getClassName(cycle.getCycleName()).replace("$", "_"); StringBuilder stringBuilder = new StringBuilder(); + stringBuilder.append("

    Cycle Map

    "); stringBuilder.append(generateGraphButtons(cycleName, dot)); if (cycle.getCycleNodes().size() + cycle.getEdgeSet().size() < dotGraphThreshold) { @@ -674,7 +676,128 @@ public String renderCycleVisuals(RankedCycle cycle, String repoUrl, CodebaseGrap return stringBuilder.toString(); } - String buildCycleDot( + String buildClassCycleDot( + Graph classGraph, + RankedCycle cycle, + String repoUrl, + CodebaseGraphDTO codebaseGraphDTO) { + StringBuilder dot = new StringBuilder(); + dot.append("`strict digraph G {\n"); + + for (DefaultWeightedEdge edge : cycle.getEdgeSet()) { + renderClassGraphEdge(classGraph, edge, dot); + } + + // render vertices + Set vertexSet = cycle.getVertexSet(); + renderClassVertices(classGraph, repoUrl, codebaseGraphDTO, vertexSet, dot); + + dot.append("}`;"); + return dot.toString(); + } + + @Override + public String renderPackageGraphVisuals(String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + String dot = buildPackageGraphDot(packageGraph, repoUrl, codebaseGraphDTO); + String packageGraphName = "packageGraph"; + + StringBuilder stringBuilder = new StringBuilder(); + stringBuilder.append("

    Package Map

    "); + stringBuilder.append(generateGraphButtons(packageGraphName, dot)); + + stringBuilder.append( + "
    Excludes packages that have no incoming and outgoing edges
    "); + + int packageCount = packageGraph.vertexSet().size(); + int relationshipCount = packageGraph.edgeSet().size(); + stringBuilder.append("
    Number of packages: " + packageCount + " Number of relationships: " + + relationshipCount + "
    "); + if (packageCount + relationshipCount < dotGraphThreshold) { + stringBuilder.append(generateDotImage(packageGraphName)); + } else { + // revisit and add DOT SVG popup button + stringBuilder.append("
    \nSVG is too big to render quickly
    \n"); + } + + return stringBuilder.toString(); + } + + String buildPackageGraphDot( + Graph packageGraph, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + StringBuilder dot = new StringBuilder(); + dot.append("`strict digraph G {\n"); + + for (DefaultWeightedEdge edge : packageGraph.edgeSet()) { + renderPackageGraphEdge(packageGraph, edge, dot); + } + + // capture only classes that have a relationship with one or more other classes + Set vertexesToRender = new HashSet<>(); + for (DefaultWeightedEdge edge : packageGraph.edgeSet()) { + String[] vertexes = extractVertexes(edge); + vertexesToRender.add(vertexes[0].trim()); + vertexesToRender.add(vertexes[1].trim()); + } + + // render vertices + renderPackageVertices(packageGraph, repoUrl, codebaseGraphDTO, vertexesToRender, dot); + + dot.append("}`;"); + return dot.toString(); + } + + private void renderPackageGraphEdge( + Graph packageGraph, DefaultWeightedEdge edge, StringBuilder dot) { + // render edge + String[] vertexes = extractVertexes(edge); + + String start = vertexes[0].trim().replace(".", "_"); + String end = vertexes[1].trim().replace(".", "_"); + + log.debug("Rendering edge: {} -> {}", start, end); + dot.append(start); + dot.append(" -> "); + dot.append(end); + + // render edge attributes + int edgeWeight = (int) packageGraph.getEdgeWeight(edge); + dot.append(" [ "); + dot.append("label = \""); + dot.append(edgeWeight); + dot.append("\" "); + dot.append("weight = \""); + dot.append(edgeWeight); + dot.append("\""); + + if (packageRelationshipsToRemove.contains(edge)) { + dot.append(" color = \"red\""); + } + + dot.append(" ];\n"); + } + + private void renderPackageVertices( + Graph classGraph, + String repoUrl, + CodebaseGraphDTO codebaseGraphDTO, + Set vertexesToRender, + StringBuilder dot) { + for (String packageName : vertexesToRender) { + dot.append(packageName.replace(".", "_")); + + dot.append(" [label=\""); + dot.append(packageName); + dot.append("\""); + + if (packagesToRemove.contains(packageName)) { + dot.append(" color=red style=filled"); + } + + dot.append("];\n"); + } + } + + String buildPackageDot( Graph classGraph, RankedCycle cycle, String repoUrl, @@ -683,7 +806,7 @@ String buildCycleDot( dot.append("`strict digraph G {\n"); for (DefaultWeightedEdge edge : cycle.getEdgeSet()) { - renderEdge(classGraph, edge, dot); + renderClassGraphEdge(classGraph, edge, dot); } // render vertices diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java index 8eb72cdc..1b656571 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java @@ -298,6 +298,10 @@ public StringBuilder generateReport( stringBuilder.append("
    \n" + "
    \n" + "
    \n" + "
    \n" + "
    \n" + "
    \n" + "
    \n"); } + stringBuilder.append(renderPackageGraphVisuals(repoUrl, codebaseGraphDTO)); + stringBuilder.append("
    \n"); + stringBuilder.append(renderGithubButtons()); + if (!packageRelationshipDisharmonies.isEmpty()) { stringBuilder.append( renderPackageEdgeDisharmonies(packageRelationshipDisharmonies, repoUrl, codebaseGraphDTO)); @@ -675,7 +679,7 @@ private String renderSingleCycle(RankedCycle cycle, String repoUrl, CodebaseGrap + getClassName(cycle.getCycleName()) + "\n"); stringBuilder.append( "

    Limiting number of cycles displayed to 1 to keep page load time fast

    \n"); - stringBuilder.append(renderCycleVisuals(cycle, repoUrl, codebaseGraphDTO)); + stringBuilder.append(renderClassCycleVisuals(cycle, repoUrl, codebaseGraphDTO)); stringBuilder.append("
    "); stringBuilder.append(""); @@ -740,7 +744,11 @@ public String renderClassGraphVisuals(String repoUrl, CodebaseGraphDTO codebaseG return ""; // empty on purpose } - public String renderCycleVisuals(RankedCycle cycle, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + public String renderPackageGraphVisuals(String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + return ""; // empty on purpose + } + + public String renderClassCycleVisuals(RankedCycle cycle, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { return ""; // empty on purpose } diff --git a/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java b/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java index bc2cf60b..ad46a8cd 100644 --- a/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java +++ b/report/src/test/java/org/hjug/refactorfirst/report/HtmlReportTest.java @@ -39,7 +39,7 @@ void getDescription() { } @Test - void buildCycleDot() { + void buildClassCycleDot() { Graph classGraph = new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class); classGraph.addVertex("A"); classGraph.addVertex("B"); @@ -61,7 +61,7 @@ void buildCycleDot() { map.put("C", "/src/main/java/org/hjug/refactorfirst/C.java"); when(dto.getClassToSourceFilePathMapping()).thenReturn(map); String repoUrl = "https://github.com/refactorfirst/RefactorFirst/blob"; - String dot = htmlReport.buildCycleDot(classGraph, rankedCycle, repoUrl, dto); + String dot = htmlReport.buildClassCycleDot(classGraph, rankedCycle, repoUrl, dto); String expectedDot = "`strict digraph G {\n" + "A -> B [ label = \"2\" weight = \"2\" ];\n" + "B -> C [ label = \"1\" weight = \"1\" ];\n" From 6bf3592bdc106a222cca92c1c71de7c47d2c8712 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Thu, 18 Jun 2026 08:03:16 -0500 Subject: [PATCH 5/9] #185 Remove unused method --- .../hjug/refactorfirst/report/HtmlReport.java | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java index d8a2cb7d..d9063f17 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/HtmlReport.java @@ -797,26 +797,6 @@ private void renderPackageVertices( } } - String buildPackageDot( - Graph classGraph, - RankedCycle cycle, - String repoUrl, - CodebaseGraphDTO codebaseGraphDTO) { - StringBuilder dot = new StringBuilder(); - dot.append("`strict digraph G {\n"); - - for (DefaultWeightedEdge edge : cycle.getEdgeSet()) { - renderClassGraphEdge(classGraph, edge, dot); - } - - // render vertices - Set vertexSet = cycle.getVertexSet(); - renderClassVertices(classGraph, repoUrl, codebaseGraphDTO, vertexSet, dot); - - dot.append("}`;"); - return dot.toString(); - } - String generate2DPopup(String cycleName) { // Created by generative AI and modified return "