diff --git a/README.md b/README.md index 7f607da6..5be6f239 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ It scans your Git repository generates a single page application by runing: Code map viewers are powered by [3D Force Graph](https://vasturiano.github.io/3d-force-graph), [sigma.js](https://www.sigmajs.org/), and [GraphViz DOT](https://graphviz.org/docs/layouts/dot/)
If there are more than 4000 classes + relationships, a simplified 3D viewer will be used to avoid slowdowns. Features will be toggleable in the 3D UI in a future release. -Take a look at the [Spring Petclinic REST project sample report](https://rawcdn.githack.com/refactorfirst/RefactorFirst/035e141f7a42920a32d96f74e819ad370fece5e7/spring-petclinic-rest-report.html)! +Take a look at the [Spring Petclinic REST project sample report](https://rawcdn.githack.com/refactorfirst/RefactorFirst/c46d26211a91ffbe08d4089e04a85ff31eb093c0/spring-petclinic-rest-report.html)! The graphs generated in the report will look similar to this one: ![image info](./RefactorFirst_Sample_Report.png) @@ -32,7 +32,7 @@ If you use an old JDK release of your chosen Java version, you may encounter iss Run the following command from the root of your project (the source code does not need to be built): ```bash -mvn org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.7.0:htmlReport +mvn org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.7.1:htmlReport ``` View the report at ```target/site/refactor-first-report.html``` @@ -40,7 +40,7 @@ View the report at ```target/site/refactor-first-report.html``` This will generate a simplified HTML report (no graphs or images) as the output of a GitHub Action step ```bash mvn -B clean test \ -org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.7.0:simpleHtmlReport \ +org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.7.1:simpleHtmlReport \ && echo "$(cat target/site/refactor-first-report.html)" >> $GITHUB_STEP_SUMMARY ``` @@ -53,7 +53,7 @@ Add the following to your project in the build section. **showDetails** will sh org.hjug.refactorfirst.plugin refactor-first-maven-plugin - 0.7.0 + 0.7.1 false @@ -74,7 +74,7 @@ A RefactorFirst report will show up in the site report when you run ```mvn site` org.hjug.refactorfirst.plugin refactor-first-maven-plugin - 0.7.0 + 0.7.1 ... @@ -139,7 +139,7 @@ I would like to create a Gradle plugin and (possibly) support non-conventional p and then (assuming Maven is installed) run ```bash -mvn org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.7.0:htmlReport +mvn org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.7.1:htmlReport ``` ## Viewing the Report @@ -165,7 +165,7 @@ There is still much to be done. Your feedback and collaboration would be greatl If you find this plugin useful, please star this repository and share with your friends & colleagues and on social media. ## Future Plans -* Improve class cycle analysis (only field member types and method signature types are currently supported). +* Improve class cycle analysis * Add a Gradle plugin. * Incorporate Unit Test coverage metrics to quickly identify the safety of refactoring classes. * Incorporate bug counts per class to the Impact (Y-Axis) calculation. diff --git a/change-proneness-ranker/pom.xml b/change-proneness-ranker/pom.xml index 610343c2..b2a82968 100644 --- a/change-proneness-ranker/pom.xml +++ b/change-proneness-ranker/pom.xml @@ -5,7 +5,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.changepronenessranker 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 0add55ae..edcb4817 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 @@ -48,61 +48,11 @@ public void close() throws Exception { git.close(); } - // Based on - // https://github.com/Cosium/git-code-format-maven-plugin/blob/master/src/main/java/com/cosium/code/format/AbstractMavenGitCodeFormatMojo.java - // MIT License - // Move to a provider? - public Repository gitRepository(File basedir) throws IOException { - Repository gitRepository; - FileRepositoryBuilder repositoryBuilder = new FileRepositoryBuilder().findGitDir(basedir); - String gitIndexFileEnvVariable = System.getenv("GIT_INDEX_FILE"); - if (Objects.nonNull(gitIndexFileEnvVariable) - && !gitIndexFileEnvVariable.trim().isEmpty()) { - log.debug("Setting Index File based on Env Variable GIT_INDEX_FILE {}", gitIndexFileEnvVariable); - repositoryBuilder = repositoryBuilder.setIndexFile(new File(gitIndexFileEnvVariable)); - } - gitRepository = repositoryBuilder.build(); - - return gitRepository; - } - public File getGitDir(File basedir) { FileRepositoryBuilder repositoryBuilder = new FileRepositoryBuilder().findGitDir(basedir); return repositoryBuilder.getGitDir(); } - // https://stackoverflow.com/a/19950970/346247 - // and - // https://github.com/centic9/jgit-cookbook/blob/master/src/main/java/org/dstadler/jgit/api/ReadFileFromCommit.java - public Map listRepositoryContentsAtHEAD(Repository repository) throws IOException { - Ref head = repository.exactRef("HEAD"); - // a RevWalk allows us to walk over commits based on some filtering that is defined - RevWalk walk = new RevWalk(repository); - RevCommit commit = walk.parseCommit(head.getObjectId()); - RevTree tree = commit.getTree(); - - TreeWalk treeWalk = new TreeWalk(repository); - treeWalk.addTree(tree); - treeWalk.setRecursive(false); - - // TODO: extract rest of this method to test it - Map fileContentsCollection = new HashMap<>(); - while (treeWalk.next()) { - if (treeWalk.isSubtree()) { - treeWalk.enterSubtree(); - } else { - if (treeWalk.getPathString().endsWith(JAVA_FILE_TYPE)) { - ObjectId objectId = treeWalk.getObjectId(0); - ObjectLoader loader = repository.open(objectId); - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - loader.copyTo(outputStream); - fileContentsCollection.put(treeWalk.getPathString(), outputStream); - } - } - } - return fileContentsCollection; - } - // log --follow implementation may be worth adopting in the future // https://github.com/spearce/jgit/blob/master/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java diff --git a/cli/pom.xml b/cli/pom.xml index 34078cb4..d64e1a23 100644 --- a/cli/pom.xml +++ b/cli/pom.xml @@ -4,7 +4,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT jar diff --git a/codebase-graph-builder/pom.xml b/codebase-graph-builder/pom.xml index c50443bd..ed4f95a0 100644 --- a/codebase-graph-builder/pom.xml +++ b/codebase-graph-builder/pom.xml @@ -5,7 +5,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.codebasegraphbuilder diff --git a/cost-benefit-calculator/pom.xml b/cost-benefit-calculator/pom.xml index 495cf464..947a51b4 100644 --- a/cost-benefit-calculator/pom.xml +++ b/cost-benefit-calculator/pom.xml @@ -5,7 +5,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.costbenefitcalculator diff --git a/coverage/pom.xml b/coverage/pom.xml index 2145aec1..e95c80d9 100644 --- a/coverage/pom.xml +++ b/coverage/pom.xml @@ -7,7 +7,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT coverage diff --git a/dsm/pom.xml b/dsm/pom.xml index 7825559b..176ca053 100644 --- a/dsm/pom.xml +++ b/dsm/pom.xml @@ -5,7 +5,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.dsm diff --git a/dsm/src/main/java/org/hjug/dsm/DSM.java b/dsm/src/main/java/org/hjug/dsm/DSM.java index 618ecae0..bfd5ffa5 100644 --- a/dsm/src/main/java/org/hjug/dsm/DSM.java +++ b/dsm/src/main/java/org/hjug/dsm/DSM.java @@ -2,6 +2,7 @@ import java.util.*; import java.util.stream.Collectors; + import lombok.Getter; import org.jgrapht.Graph; import org.jgrapht.Graphs; @@ -34,9 +35,16 @@ */ public class DSM { - private Graph graph; + private final Graph graph; private List sortedActivities; boolean activitiesSorted = false; + private final List edgesAboveDiagonal = new ArrayList<>(); + + List sparseIntSortedActivities; + SparseIntDirectedWeightedGraph sparseGraph; + + @Getter + double sumOfEdgeWeightsAboveDiagonal; Map vertexToInt = new HashMap<>(); Map intToVertex = new HashMap<>(); @@ -68,13 +76,14 @@ public void addDependency(String from, String to, int weight) { } private void orderVertices() { - SparseIntDirectedWeightedGraph sparseGraph = getSparseIntDirectedWeightedGraph(); - List> sccs = findStronglyConnectedComponents(sparseGraph); - sortedActivities = convertIntToStringVertices(topologicalSort(sccs, sparseGraph)); + sparseGraph = getSparseIntDirectedWeightedGraph(); + List> sccs = this.findStronglyConnectedSparseGraphComponents(sparseGraph); + sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); // reversing corrects rendering of the DSM // with sources as rows and targets as columns // was needed after AI solution was generated and iterated - Collections.reverse(sortedActivities); + Collections.reverse(sparseIntSortedActivities); + sortedActivities = convertIntToStringVertices(sparseIntSortedActivities); activitiesSorted = true; } @@ -104,23 +113,24 @@ List convertIntToStringVertices(List intVertices) { /** * Kosaraju SCC detector avoids stack overflow. * It is used by JGraphT's CycleDetector, and makes sense to use it here as well for consistency + * * @param graph * @return */ - private List> findStronglyConnectedComponents(Graph graph) { + private List> findStronglyConnectedSparseGraphComponents(Graph graph) { KosarajuStrongConnectivityInspector kosaraju = new KosarajuStrongConnectivityInspector<>(graph); return kosaraju.stronglyConnectedSets(); } - private List topologicalSort(List> sccs, Graph graph) { + private List topologicalSortSparseGraph(List> sccs, Graph graph) { List sortedActivities = new ArrayList<>(); Set visited = new HashSet<>(); for (Set scc : sccs) { for (Integer activity : scc) { if (!visited.contains(activity)) { - topologicalSortUtil(activity, visited, sortedActivities, graph); + topologicalSortUtilSparseGraph(activity, visited, sortedActivities, graph); } } } @@ -129,13 +139,13 @@ private List topologicalSort(List> sccs, Graph visited, List sortedActivities, Graph graph) { visited.add(activity); for (Integer neighbor : Graphs.successorListOf(graph, activity)) { if (!visited.contains(neighbor)) { - topologicalSortUtil(neighbor, visited, sortedActivities, graph); + topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph); } } @@ -147,20 +157,45 @@ public List getEdgesAboveDiagonal() { orderVertices(); } - List edgesAboveDiagonal = new ArrayList<>(); + if (edgesAboveDiagonal.isEmpty()) { + for (int i = 0; i < sortedActivities.size(); i++) { + for (int j = i + 1; j < sortedActivities.size(); j++) { + // source / destination vertex was flipped after solution generation + // to correctly identify the vertex above the diagonal to remove + DefaultWeightedEdge edge = graph.getEdge(sortedActivities.get(i), sortedActivities.get(j)); + if (edge != null) { + edgesAboveDiagonal.add(edge); + } + } + } - for (int i = 0; i < sortedActivities.size(); i++) { - for (int j = i + 1; j < sortedActivities.size(); j++) { + sumOfEdgeWeightsAboveDiagonal = edgesAboveDiagonal.stream() + .mapToInt(edge -> (int) graph.getEdgeWeight(edge)).sum(); + } + + return edgesAboveDiagonal; + } + + private List getSparseEdgesAboveDiagonal() { + if (!activitiesSorted) { + orderVertices(); + } + + List sparseEdgesAboveDiagonal = new ArrayList<>(); + + for (int i = 0; i < sparseIntSortedActivities.size(); i++) { + for (int j = i + 1; j < sparseIntSortedActivities.size(); j++) { // source / destination vertex was flipped after solution generation // to correctly identify the vertex above the diagonal to remove - DefaultWeightedEdge edge = graph.getEdge(sortedActivities.get(i), sortedActivities.get(j)); + Integer edge = sparseGraph.getEdge(sparseIntSortedActivities.get(i), sparseIntSortedActivities.get(j)); + if (edge != null) { - edgesAboveDiagonal.add(edge); + sparseEdgesAboveDiagonal.add(edge); } } } - return edgesAboveDiagonal; + return sparseEdgesAboveDiagonal; } public DefaultWeightedEdge getFirstLowestWeightEdgeAboveDiagonalToRemove() { @@ -243,32 +278,60 @@ void printDSM(Graph graph, List sortedActiv } } + // TODO: Delete all code below this line + // Will be superseded by Minimum Feedback Arc + Vertex calculations + ///////////////////////////////////////////////////////// + // "Standard" Graph implementation to find edge to remove + ///////////////////////////////////////////////////////// + /** * Captures the impact of the removal of each edge above the diagonal. */ - public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) { - // get edges above diagonal for DSM graph - List edgesAboveDiagonal; - List allEdgesAboveDiagonal = getEdgesAboveDiagonal(); - - if (limit == 0 || allEdgesAboveDiagonal.size() <= limit) { - edgesAboveDiagonal = allEdgesAboveDiagonal; - } else { - // get first 50 values of min weight - List minimumWeightEdgesAboveDiagonal = getMinimumWeightEdgesAboveDiagonal(); - int max = Math.min(minimumWeightEdgesAboveDiagonal.size(), limit); - edgesAboveDiagonal = minimumWeightEdgesAboveDiagonal.subList(0, max); - } - - double avgCycleNodeCount = getAverageCycleNodeCount(); + public List getImpactOfEdgesAboveDiagonalIfRemoved() { + +// // get edges above diagonal for DSM graph +// List edgesAboveDiagonal; +// List allEdgesAboveDiagonal = getEdgesAboveDiagonal(); +// +// if (limit == 0 || allEdgesAboveDiagonal.size() <= limit) { +// edgesAboveDiagonal = allEdgesAboveDiagonal; +// } else { +// // get first 50 values of min weight +// List minimumWeightEdgesAboveDiagonal = getMinimumWeightEdgesAboveDiagonal(); +// int max = Math.min(minimumWeightEdgesAboveDiagonal.size(), limit); +// edgesAboveDiagonal = minimumWeightEdgesAboveDiagonal.subList(0, max); +// } + + int currentCycleCount = new CircularReferenceChecker().getCycles(graph).size(); + + return getEdgesAboveDiagonal().stream() + .map(this::calculateEdgeToRemoveInfo) + .sorted(Comparator + .comparing((EdgeToRemoveInfo edgeToRemoveInfo) -> currentCycleCount - edgeToRemoveInfo.getNewCycleCount()) + /*.thenComparing(EdgeToRemoveInfo::getEdgeWeight)*/) + .collect(Collectors.toList()); + } - // build the cloned graph - Graph clonedGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); - graph.vertexSet().forEach(clonedGraph::addVertex); + private EdgeToRemoveInfo calculateEdgeToRemoveInfo(DefaultWeightedEdge edgeToRemove) { + //clone graph and remove edge + Graph improvedGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + graph.vertexSet().forEach(improvedGraph::addVertex); for (DefaultWeightedEdge weightedEdge : graph.edgeSet()) { - clonedGraph.addEdge(graph.getEdgeSource(weightedEdge), graph.getEdgeTarget(weightedEdge), weightedEdge); + improvedGraph.addEdge(graph.getEdgeSource(weightedEdge), graph.getEdgeTarget(weightedEdge), weightedEdge); } + improvedGraph.removeEdge(edgeToRemove); + + // Calculate new cycle count + int newCycleCount = new CircularReferenceChecker().getCycles(improvedGraph).size(); + + //calculate new graph statistics + double removedEdgeWeight = graph.getEdgeWeight(edgeToRemove); + double payoff = newCycleCount / removedEdgeWeight; + return new EdgeToRemoveInfo(edgeToRemove, (int) removedEdgeWeight, newCycleCount, payoff); + } + + /*public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) { List edgesToRemove = new ArrayList<>(); // capture impact of each edge on graph when removed for (DefaultWeightedEdge edge : edgesAboveDiagonal) { @@ -284,7 +347,7 @@ public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) // identify updated cycles and calculate updated graph information edgesToRemove.add(getEdgeToRemoveInfo( - edge, edgeInCyclesCount, avgCycleNodeCount, new CircularReferenceChecker().getCycles(clonedGraph))); + edge, edgeInCyclesCount, new CircularReferenceChecker().getCycles(clonedGraph))); // add the edge back for next iteration clonedGraph.addEdge(graph.getEdgeSource(edge), graph.getEdgeTarget(edge), edge); @@ -294,38 +357,134 @@ public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) edgesToRemove.sort(Comparator.comparing(EdgeToRemoveInfo::getPayoff)); Collections.reverse(edgesToRemove); return edgesToRemove; + }*/ + + public List getEdgesAboveDiagonal(Graph graph, List sortedActivities) { + List edgesAboveDiagonal = new ArrayList<>(); + for (int i = 0; i < sortedActivities.size(); i++) { + for (int j = i + 1; j < sortedActivities.size(); j++) { + // source / destination vertex was flipped after solution generation + // to correctly identify the vertex above the diagonal to remove + DefaultWeightedEdge edge = graph.getEdge(sortedActivities.get(i), sortedActivities.get(j)); + if (edge != null) { + edgesAboveDiagonal.add(edge); + } + } + } + + return edgesAboveDiagonal; + } + + private List orderVertices(Graph graph) { + List> sccs = findStronglyConnectedComponents(graph); + List sparseIntSortedActivities = topologicalSort(sccs, graph); + // reversing corrects rendering of the DSM + // with sources as rows and targets as columns + // was needed after AI solution was generated and iterated + Collections.reverse(sparseIntSortedActivities); + + return sparseIntSortedActivities; + } + + private List topologicalSort(List> sccs, Graph graph) { + List sortedActivities = new ArrayList<>(); + Set visited = new HashSet<>(); + + for (Set scc : sccs) { + for (String activity : scc) { + if (!visited.contains(activity)) { + topologicalSortUtil(activity, visited, sortedActivities, graph); + } + } + } + + Collections.reverse(sortedActivities); + return sortedActivities; + } + + private void topologicalSortUtil( + String activity, Set visited, List sortedActivities, Graph graph) { + visited.add(activity); + + for (String neighbor : Graphs.successorListOf(graph, activity)) { + if (!visited.contains(neighbor)) { + topologicalSortUtil(neighbor, visited, sortedActivities, graph); + } + } + + sortedActivities.add(activity); } - private EdgeToRemoveInfo getEdgeToRemoveInfo( - DefaultWeightedEdge edge, - int edgeInCyclesCount, - double currentAvgCycleNodeCount, - Map> cycles) { - // get the new number of cycles - int newCycleCount = cycles.size(); + private List> findStronglyConnectedComponents(Graph graph) { + KosarajuStrongConnectivityInspector kosaraju = + new KosarajuStrongConnectivityInspector<>(graph); + return kosaraju.stronglyConnectedSets(); + } - // calculate the average cycle node count - double newAverageCycleNodeCount = getAverageCycleNodeCount(cycles); + ///////////////////////////////////////////////////////// + // Sparse Int Graph implementation to find edge to remove + ///////////////////////////////////////////////////////// - // capture the what-if values - double edgeWeight = graph.getEdgeWeight(edge); + public List getImpactOfSparseEdgesAboveDiagonalIfRemoved() { + List sparseEdgesAboveDiagonal = getSparseEdgesAboveDiagonal(); + return sparseEdgesAboveDiagonal.stream() + .map(this::calculateSparseEdgeToRemoveInfo) + .sorted(Comparator.comparing(EdgeToRemoveInfo::getPayoff).thenComparing(EdgeToRemoveInfo::getRemovedEdgeWeight)) + .collect(Collectors.toList()); + } - double impact = (currentAvgCycleNodeCount - newAverageCycleNodeCount) / edgeWeight; - return new EdgeToRemoveInfo( - edge, edgeWeight, edgeInCyclesCount, newCycleCount, newAverageCycleNodeCount, impact); + private EdgeToRemoveInfo calculateSparseEdgeToRemoveInfo(Integer edgeToRemove) { + //clone graph and remove edge + int source = sparseGraph.getEdgeSource(edgeToRemove); + int target = sparseGraph.getEdgeTarget(edgeToRemove); + double weight = sparseGraph.getEdgeWeight(edgeToRemove); + Triple removedEdge = Triple.of(source, target, weight); + + List> updatedEdgeList = new ArrayList<>(sparseEdges); + updatedEdgeList.remove(removedEdge); + + SparseIntDirectedWeightedGraph improvedGraph = new SparseIntDirectedWeightedGraph(vertexCount, updatedEdgeList); + + // find edges above diagonal + List sortedSparseActivities = orderVertices(improvedGraph); + List updatedEdges = getSparseEdgesAboveDiagonal(improvedGraph, sortedSparseActivities); + + // calculate new graph statistics + int newEdgeCount = updatedEdges.size(); + double newEdgeWeightSum = updatedEdges.stream() + .mapToDouble(improvedGraph::getEdgeWeight).sum(); + DefaultWeightedEdge defaultWeightedEdge = + graph.getEdge(intToVertex.get(source), intToVertex.get(target)); + double payoff = (sumOfEdgeWeightsAboveDiagonal - newEdgeWeightSum) / weight; + return new EdgeToRemoveInfo(defaultWeightedEdge, (int) weight, newEdgeCount, payoff); } - public static double getAverageCycleNodeCount(Map> cycles) { - return cycles.values().stream() - .mapToInt(cycle -> cycle.vertexSet().size()) - .average() - .orElse(0.0); + private List orderVertices(SparseIntDirectedWeightedGraph sparseGraph) { + List> sccs = this.findStronglyConnectedSparseGraphComponents(sparseGraph); + List sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); + // reversing corrects rendering of the DSM + // with sources as rows and targets as columns + // was needed after AI solution was generated and iterated + Collections.reverse(sparseIntSortedActivities); + + return sparseIntSortedActivities; } - public double getAverageCycleNodeCount() { - return cycles.values().stream() - .mapToInt(cycle -> cycle.vertexSet().size()) - .average() - .orElse(0.0); + private List getSparseEdgesAboveDiagonal(SparseIntDirectedWeightedGraph sparseGraph, List sparseIntSortedActivities) { + List sparseEdgesAboveDiagonal = new ArrayList<>(); + + for (int i = 0; i < sparseIntSortedActivities.size(); i++) { + for (int j = i + 1; j < sparseIntSortedActivities.size(); j++) { + // source / destination vertex was flipped after solution generation + // to correctly identify the vertex above the diagonal to remove + Integer edge = sparseGraph.getEdge(sparseIntSortedActivities.get(i), sparseIntSortedActivities.get(j)); + + if (edge != null) { + sparseEdgesAboveDiagonal.add(edge); + } + } + } + + return sparseEdgesAboveDiagonal; } } diff --git a/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java b/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java index c16e0967..f284216b 100644 --- a/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java +++ b/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java @@ -6,9 +6,7 @@ @Data public class EdgeToRemoveInfo { private final DefaultWeightedEdge edge; - private final double edgeWeight; - private final int edgeInCycleCount; + private final int removedEdgeWeight; private final int newCycleCount; - private final double averageCycleNodeCount; private final double payoff; // impact / effort } diff --git a/dsm/src/main/java/org/hjug/dsm/OptimalBackEdgeRemover.java b/dsm/src/main/java/org/hjug/dsm/OptimalBackEdgeRemover.java new file mode 100644 index 00000000..598396a9 --- /dev/null +++ b/dsm/src/main/java/org/hjug/dsm/OptimalBackEdgeRemover.java @@ -0,0 +1,105 @@ +package org.hjug.dsm; + +import org.jgrapht.Graph; +import org.jgrapht.alg.cycle.CycleDetector; +import org.jgrapht.alg.cycle.JohnsonSimpleCycles; +import org.jgrapht.graph.AsSubgraph; + +import java.util.*; + +public class OptimalBackEdgeRemover { + private Graph graph; + + /** + * Constructor initializing with the target graph. + * @param graph The directed weighted graph to analyze + */ + public OptimalBackEdgeRemover(Graph graph) { + this.graph = graph; + } + + /** + * Finds the optimal back edge(s) to remove to move the graph closer to a DAG. + * @return A set of edges to remove + */ + public Set findOptimalBackEdgesToRemove() { + CycleDetector cycleDetector = new CycleDetector<>(graph); + + // If the graph is already acyclic, return empty set + if (!cycleDetector.detectCycles()) { + return Collections.emptySet(); + } + + // Find all cycles in the graph + JohnsonSimpleCycles cycleFinder = new JohnsonSimpleCycles<>(graph); + List> originalCycles = cycleFinder.findSimpleCycles(); + int originalCycleCount = originalCycles.size(); + + // Identify edges that are part of at least one cycle + Set edgesInCycles = new HashSet<>(); + for (List cycle : originalCycles) { + for (int i = 0; i < cycle.size(); i++) { + V source = cycle.get(i); + V target = cycle.get((i + 1) % cycle.size()); + E edge = graph.getEdge(source, target); + edgesInCycles.add(edge); + } + } + + // Calculate cycle elimination count for each edge + Map edgeCycleEliminationCount = new HashMap<>(); + for (E edge : edgesInCycles) { + // Create a subgraph without this edge + Graph subgraph = new AsSubgraph<>(graph, graph.vertexSet(), new HashSet<>(graph.edgeSet())); + subgraph.removeEdge(edge); + + // Calculate how many cycles would be eliminated + JohnsonSimpleCycles subgraphCycleFinder = new JohnsonSimpleCycles<>(subgraph); + List> remainingCycles = subgraphCycleFinder.findSimpleCycles(); + int cyclesEliminated = originalCycleCount - remainingCycles.size(); + + edgeCycleEliminationCount.put(edge, cyclesEliminated); + } + + // Find edges that eliminate the most cycles + int maxCycleElimination = 0; + List maxEliminationEdges = new ArrayList<>(); + + for (Map.Entry entry : edgeCycleEliminationCount.entrySet()) { + if (entry.getValue() > maxCycleElimination) { + maxCycleElimination = entry.getValue(); + maxEliminationEdges.clear(); + maxEliminationEdges.add(entry.getKey()); + } else if (entry.getValue() == maxCycleElimination) { + maxEliminationEdges.add(entry.getKey()); + } + } + + // If no cycles are eliminated (shouldn't happen), return empty set + if (maxEliminationEdges.isEmpty() || maxCycleElimination == 0) { + return Collections.emptySet(); + } + + // If multiple edges eliminate the same number of cycles, choose the one with the lowest weight + if (maxEliminationEdges.size() > 1) { + double minWeight = Double.MAX_VALUE; + List minWeightEdges = new ArrayList<>(); + + for (E edge : maxEliminationEdges) { + double weight = graph.getEdgeWeight(edge); + if (weight < minWeight) { + minWeight = weight; + minWeightEdges.clear(); + minWeightEdges.add(edge); + } else if (weight == minWeight) { + minWeightEdges.add(edge); + } + } + + return new HashSet<>(minWeightEdges); + } + + // Return the single edge that eliminates the most cycles + return new HashSet<>(maxEliminationEdges); + } +} diff --git a/dsm/src/main/java/org/hjug/dsm/SparseGraphCircularReferenceChecker.java b/dsm/src/main/java/org/hjug/dsm/SparseGraphCircularReferenceChecker.java new file mode 100644 index 00000000..926439ad --- /dev/null +++ b/dsm/src/main/java/org/hjug/dsm/SparseGraphCircularReferenceChecker.java @@ -0,0 +1,71 @@ +package org.hjug.dsm; + +import lombok.extern.slf4j.Slf4j; +import org.jgrapht.alg.cycle.CycleDetector; +import org.jgrapht.graph.AsSubgraph; +import org.jgrapht.opt.graph.sparse.SparseIntDirectedWeightedGraph; + +import java.util.HashMap; +import java.util.Map; + +@Slf4j +public class SparseGraphCircularReferenceChecker { + + private final Map> uniqueSubGraphs = new HashMap<>(); + + /** + * Detects cycles in the graph that is passed in + * and returns the unique cycles in the graph as a map of subgraphs + * + * @param graph + * @return a Map of unique cycles in the graph + */ + public Map> getCycles(SparseIntDirectedWeightedGraph graph) { + + if (!uniqueSubGraphs.isEmpty()) { + return uniqueSubGraphs; + } + + // use CycleDetector.findCycles()? + Map> cycles = detectCycles(graph); + + cycles.forEach((vertex, subGraph) -> { + int vertexCount = subGraph.vertexSet().size(); + int edgeCount = subGraph.edgeSet().size(); + + if (vertexCount > 1 && edgeCount > 1 && !isDuplicateSubGraph(subGraph, vertex)) { + uniqueSubGraphs.put(vertex, subGraph); + log.debug("Vertex: {} vertex count: {} edge count: {}", vertex, vertexCount, edgeCount); + } + }); + + return uniqueSubGraphs; + } + + private boolean isDuplicateSubGraph(AsSubgraph subGraph, Integer vertex) { + if (!uniqueSubGraphs.isEmpty()) { + for (AsSubgraph renderedSubGraph : uniqueSubGraphs.values()) { + if (renderedSubGraph.vertexSet().size() == subGraph.vertexSet().size() + && renderedSubGraph.edgeSet().size() + == subGraph.edgeSet().size() + && renderedSubGraph.vertexSet().contains(vertex)) { + return true; + } + } + } + + return false; + } + + private Map> detectCycles( + SparseIntDirectedWeightedGraph graph) { + Map> cyclesForEveryVertexMap = new HashMap<>(); + CycleDetector cycleDetector = new CycleDetector<>(graph); + cycleDetector.findCycles().forEach(v -> { + AsSubgraph subGraph = + new AsSubgraph<>(graph, cycleDetector.findCyclesContainingVertex(v)); + cyclesForEveryVertexMap.put(v, subGraph); + }); + return cyclesForEveryVertexMap; + } +} diff --git a/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java b/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java new file mode 100644 index 00000000..01d6aa24 --- /dev/null +++ b/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java @@ -0,0 +1,180 @@ +package org.hjug.dsm; + +import org.jgrapht.Graph; +import org.jgrapht.Graphs; +import org.jgrapht.alg.connectivity.KosarajuStrongConnectivityInspector; +import org.jgrapht.alg.util.Triple; +import org.jgrapht.graph.DefaultWeightedEdge; +import org.jgrapht.opt.graph.sparse.SparseIntDirectedWeightedGraph; + +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ConcurrentSkipListSet; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +// TODO: Delete +class SparseIntDWGEdgeRemovalCalculator { + private final Graph graph; + SparseIntDirectedWeightedGraph sparseGraph; + List> sparseEdges; + List sparseEdgesAboveDiagonal; + private final double sumOfEdgeWeightsAboveDiagonal; + int vertexCount; + Map vertexToInt; + Map intToVertex; + + + SparseIntDWGEdgeRemovalCalculator( + Graph graph, + SparseIntDirectedWeightedGraph sparseGraph, + List> sparseEdges, + List sparseEdgesAboveDiagonal, + double sumOfEdgeWeightsAboveDiagonal, + int vertexCount, + Map vertexToInt, + Map intToVertex) { + this.graph = graph; + this.sparseGraph = sparseGraph; + this.sparseEdges = new CopyOnWriteArrayList<>(sparseEdges); + this.sparseEdgesAboveDiagonal = new CopyOnWriteArrayList<>(sparseEdgesAboveDiagonal); + this.sumOfEdgeWeightsAboveDiagonal = sumOfEdgeWeightsAboveDiagonal; + this.vertexCount = vertexCount; + this.vertexToInt = new ConcurrentHashMap<>(vertexToInt); + this.intToVertex = new ConcurrentHashMap<>(intToVertex); + + } + + public List getImpactOfSparseEdgesAboveDiagonalIfRemoved() { + return sparseEdgesAboveDiagonal.parallelStream() + .map(this::calculateSparseEdgeToRemoveInfo) + .sorted(Comparator.comparing(EdgeToRemoveInfo::getPayoff).thenComparing(EdgeToRemoveInfo::getRemovedEdgeWeight)) + .collect(Collectors.toList()); + } + + private EdgeToRemoveInfo calculateSparseEdgeToRemoveInfo(Integer edgeToRemove) { + //clone graph and remove edge + int source = sparseGraph.getEdgeSource(edgeToRemove); + int target = sparseGraph.getEdgeTarget(edgeToRemove); + double weight = sparseGraph.getEdgeWeight(edgeToRemove); + Triple removedEdge = Triple.of(source, target, weight); + + List> tempUpdatedEdgeList = new ArrayList<>(sparseEdges); + tempUpdatedEdgeList.remove(removedEdge); + List> updatedEdgeList = new CopyOnWriteArrayList<>(tempUpdatedEdgeList); + + SparseIntDirectedWeightedGraph improvedGraph = new SparseIntDirectedWeightedGraph(vertexCount, updatedEdgeList); + + // find edges above diagonal + List sortedSparseVertices = orderVertices(improvedGraph); + List updatedEdges = getSparseEdgesAboveDiagonal(improvedGraph, sortedSparseVertices); + + // calculate new graph statistics + int newEdgeCount = updatedEdges.size(); + double newEdgeWeightSum = updatedEdges.stream() + .mapToDouble(improvedGraph::getEdgeWeight).sum(); + DefaultWeightedEdge defaultWeightedEdge = + graph.getEdge(intToVertex.get(source), intToVertex.get(target)); + double payoff = (sumOfEdgeWeightsAboveDiagonal - newEdgeWeightSum) / weight; + return new EdgeToRemoveInfo(defaultWeightedEdge, (int) weight, newEdgeCount, payoff); + } + + private List orderVertices(SparseIntDirectedWeightedGraph sparseGraph) { + List> sccs = new CopyOnWriteArrayList<>(findStronglyConnectedSparseGraphComponents(sparseGraph)); +// List sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); + List sparseIntSortedActivities = topologicalParallelSortSparseGraph(sccs, sparseGraph); + // reversing corrects rendering of the DSM + // with sources as rows and targets as columns + // was needed after AI solution was generated and iterated + Collections.reverse(sparseIntSortedActivities); + + return new CopyOnWriteArrayList<>(sparseIntSortedActivities); + } + + /** + * Kosaraju SCC detector avoids stack overflow. + * It is used by JGraphT's CycleDetector, and makes sense to use it here as well for consistency + * + * @param graph + * @return + */ + private List> findStronglyConnectedSparseGraphComponents(Graph graph) { + KosarajuStrongConnectivityInspector kosaraju = + new KosarajuStrongConnectivityInspector<>(graph); + return kosaraju.stronglyConnectedSets(); + } + + private List topologicalSortSparseGraph(List> sccs, Graph graph) { + List sortedActivities = new ArrayList<>(); + Set visited = new HashSet<>(); + + sccs.parallelStream() + .flatMap(Set::parallelStream) + .filter(activity -> !visited.contains(activity)) + .forEach(activity -> topologicalSortUtilSparseGraph(activity, visited, sortedActivities, graph)); + + + Collections.reverse(sortedActivities); + return sortedActivities; + } + + private void topologicalSortUtilSparseGraph( + Integer activity, Set visited, List sortedActivities, Graph graph) { + visited.add(activity); + + for (Integer neighbor : Graphs.successorListOf(graph, activity)) { + if (!visited.contains(neighbor)) { + topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph); + } + } + + sortedActivities.add(activity); + } + + private List getSparseEdgesAboveDiagonal(SparseIntDirectedWeightedGraph sparseGraph, List sortedActivities) { + ConcurrentLinkedQueue sparseEdgesAboveDiagonal = new ConcurrentLinkedQueue<>(); + + int size = sortedActivities.size(); + IntStream.range(0, size).parallel().forEach(i -> { + for (int j = i + 1; j < size; j++) { + Integer edge = sparseGraph.getEdge( + sortedActivities.get(i), + sortedActivities.get(j) + ); + if (edge != null) { + sparseEdgesAboveDiagonal.add(edge); + } + } + }); + + return new ArrayList<>(sparseEdgesAboveDiagonal); + } + + private List topologicalParallelSortSparseGraph(List> sccs, Graph graph) { + ConcurrentLinkedQueue sortedActivities = new ConcurrentLinkedQueue<>(); + Set visited = new ConcurrentSkipListSet<>(); + + sccs.parallelStream() + .flatMap(Set::parallelStream) + .filter(activity -> !visited.contains(activity)) + .forEach(activity -> topologicalSortUtilSparseGraph(activity, visited, sortedActivities, graph)); + + ArrayList sortedActivitiesList = new ArrayList<>(sortedActivities); + Collections.reverse(sortedActivitiesList); + return sortedActivitiesList; + } + + private void topologicalSortUtilSparseGraph( + Integer activity, Set visited, ConcurrentLinkedQueue sortedActivities, Graph graph) { + visited.add(activity); + + Graphs.successorListOf(graph, activity).parallelStream() + .filter(neighbor -> !visited.contains(neighbor)) + .forEach(neighbor -> topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph)); + + sortedActivities.add(activity); + } + +} diff --git a/dsm/src/test/java/org/hjug/dsm/CircularReferenceCheckerTests.java b/dsm/src/test/java/org/hjug/dsm/CircularReferenceCheckerTests.java index fb2e8439..a550278f 100644 --- a/dsm/src/test/java/org/hjug/dsm/CircularReferenceCheckerTests.java +++ b/dsm/src/test/java/org/hjug/dsm/CircularReferenceCheckerTests.java @@ -23,9 +23,16 @@ void detectCyclesTest() { classReferencesGraph.addVertex("C"); classReferencesGraph.addEdge("A", "B"); classReferencesGraph.addEdge("B", "C"); - classReferencesGraph.addEdge("C", "A"); + Map> cyclesForEveryVertexMap = sutCircularReferenceChecker.getCycles(classReferencesGraph); - assertEquals(1, cyclesForEveryVertexMap.size()); + assertEquals(0, cyclesForEveryVertexMap.size(), "Not expecting any circular references at this point"); + + classReferencesGraph.addEdge("C", "A"); + + cyclesForEveryVertexMap = sutCircularReferenceChecker.getCycles(classReferencesGraph); + assertEquals(1, cyclesForEveryVertexMap.size(), "Now we expect one circular reference"); + assertEquals("([A, B, C], [(A,B), (B,C), (C,A)])", cyclesForEveryVertexMap.get("A").toString(), + "Expected a different circular reference"); } } diff --git a/dsm/src/test/java/org/hjug/dsm/DSMTest.java b/dsm/src/test/java/org/hjug/dsm/DSMTest.java index 4e871eb1..bdc3242f 100644 --- a/dsm/src/test/java/org/hjug/dsm/DSMTest.java +++ b/dsm/src/test/java/org/hjug/dsm/DSMTest.java @@ -129,6 +129,5 @@ void getImpactOfEdgesAboveDiagonalIfRemoved() { assertEquals("(H : E)", infos.get(0).getEdge().toString()); assertEquals(2, infos.get(0).getNewCycleCount()); - assertEquals(4.5, infos.get(0).getAverageCycleNodeCount()); } } diff --git a/dsm/src/test/java/org/hjug/dsm/OptimalBackEdgeRemoverTest.java b/dsm/src/test/java/org/hjug/dsm/OptimalBackEdgeRemoverTest.java new file mode 100644 index 00000000..5eb9a0d8 --- /dev/null +++ b/dsm/src/test/java/org/hjug/dsm/OptimalBackEdgeRemoverTest.java @@ -0,0 +1,102 @@ +package org.hjug.dsm; + +import org.jgrapht.Graph; +import org.jgrapht.graph.DefaultWeightedEdge; +import org.jgrapht.graph.SimpleDirectedWeightedGraph; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.*; + +class OptimalBackEdgeRemoverTest { + + @Test + void noOptimalEdge() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + assertTrue(optimalEdges.isEmpty()); + } + + + @Test + void oneBackEdge() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + classReferencesGraph.addEdge("C", "A"); + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + // all are considered back edges since this is a cycle + assertEquals(3, optimalEdges.size()); + } + + @Test + void twoBackEdges() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addVertex("D"); + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + classReferencesGraph.addEdge("C", "D"); + classReferencesGraph.addEdge("C", "A"); // back edge + classReferencesGraph.addEdge("D", "A"); // back edge + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + assertEquals(2, optimalEdges.size()); + } + + @Test + void multi() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addVertex("D"); + + // Cycle 1 + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + classReferencesGraph.addEdge("C", "D"); + classReferencesGraph.addEdge("B", "A"); // Adding a cycle + classReferencesGraph.addEdge("C", "A"); // Adding a cycle + classReferencesGraph.addEdge("D", "A"); // Adding a cycle + + // Cycle 2 + classReferencesGraph.addVertex("E"); + classReferencesGraph.addVertex("F"); + classReferencesGraph.addVertex("G"); + classReferencesGraph.addVertex("H"); + classReferencesGraph.addEdge("E", "F"); + classReferencesGraph.addEdge("F", "G"); + classReferencesGraph.addEdge("G", "H"); + classReferencesGraph.addEdge("H", "E"); // create cycle + + classReferencesGraph.addEdge("A", "E"); + classReferencesGraph.addEdge("E", "A"); // create cycle between cycles + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + assertEquals(1, optimalEdges.size()); + assertEquals("E:A", new ArrayList<>(optimalEdges).get(0).toString()); + } +} \ No newline at end of file diff --git a/effort-ranker/pom.xml b/effort-ranker/pom.xml index 43a965a6..5679bfde 100644 --- a/effort-ranker/pom.xml +++ b/effort-ranker/pom.xml @@ -5,7 +5,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.effortranker diff --git a/graph-data-generator/pom.xml b/graph-data-generator/pom.xml index e67391fa..0ea9f753 100644 --- a/graph-data-generator/pom.xml +++ b/graph-data-generator/pom.xml @@ -5,7 +5,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.graphdatagenerator diff --git a/pom.xml b/pom.xml index 87ae8f94..aa6ac31f 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT pom https://github.com/refactorfirst/RefactorFirst @@ -390,7 +390,7 @@ com.diffplug.spotless spotless-maven-plugin - 2.30.0 + 2.44.5 diff --git a/refactor-first-maven-plugin/pom.xml b/refactor-first-maven-plugin/pom.xml index e6305837..a8492eee 100644 --- a/refactor-first-maven-plugin/pom.xml +++ b/refactor-first-maven-plugin/pom.xml @@ -5,7 +5,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.plugin diff --git a/report/pom.xml b/report/pom.xml index 21b3b239..6e4059df 100644 --- a/report/pom.xml +++ b/report/pom.xml @@ -4,7 +4,7 @@ org.hjug.refactorfirst refactor-first - 0.7.1-SNAPSHOT + 0.7.2-SNAPSHOT org.hjug.refactorfirst.report 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 b96d16f5..15888825 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java @@ -14,7 +14,6 @@ import java.util.Locale; import java.util.Optional; import lombok.extern.slf4j.Slf4j; -import org.hjug.cbc.CostBenefitCalculator; import org.hjug.cbc.CycleRanker; import org.hjug.cbc.RankedCycle; import org.hjug.cbc.RankedDisharmony; @@ -193,14 +192,14 @@ public StringBuilder generateReport( List rankedGodClassDisharmonies = List.of(); List rankedCBODisharmonies = List.of(); log.info("Identifying Object Oriented Disharmonies"); - try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir)) { - costBenefitCalculator.runPmdAnalysis(); - rankedGodClassDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues(); - rankedCBODisharmonies = costBenefitCalculator.calculateCBOCostBenefitValues(); - } catch (Exception e) { - log.error("Error running analysis."); - throw new RuntimeException(e); - } + // try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir)) { + // costBenefitCalculator.runPmdAnalysis(); + // rankedGodClassDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues(); + // rankedCBODisharmonies = costBenefitCalculator.calculateCBOCostBenefitValues(); + // } catch (Exception e) { + // log.error("Error running analysis."); + // throw new RuntimeException(e); + // } CycleRanker cycleRanker = new CycleRanker(projectBaseDir); List rankedCycles = List.of(); @@ -216,10 +215,10 @@ public StringBuilder generateReport( edgesAboveDiagonal = dsm.getEdgesAboveDiagonal(); log.info("Performing edge removal what-if analysis"); - List edgeToRemoveInfos = dsm.getImpactOfEdgesAboveDiagonalIfRemoved(edgeAnalysisCount); +// List edgeToRemoveInfos = dsm.getImpactOfSparseEdgesAboveDiagonalIfRemoved(); - if (edgeToRemoveInfos.isEmpty() - && rankedGodClassDisharmonies.isEmpty() + if (/*edgeToRemoveInfos.isEmpty() + &&*/ rankedGodClassDisharmonies.isEmpty() && rankedCBODisharmonies.isEmpty() && rankedCycles.isEmpty()) { stringBuilder @@ -233,10 +232,10 @@ public StringBuilder generateReport( return stringBuilder; } - if (!edgeToRemoveInfos.isEmpty()) { - stringBuilder.append("Back Edges\n"); - stringBuilder.append("
\n"); - } +// if (!edgeToRemoveInfos.isEmpty()) { +// stringBuilder.append("Back Edges\n"); +// stringBuilder.append("
\n"); +// } if (!rankedGodClassDisharmonies.isEmpty()) { stringBuilder.append("God Classes\n"); @@ -260,13 +259,13 @@ public StringBuilder generateReport( // Display impact of each edge if removed stringBuilder.append("
\n"); - String edgeInfos = renderEdgeToRemoveInfos(edgeToRemoveInfos); - - if (!edgeToRemoveInfos.isEmpty()) { - stringBuilder.append(edgeInfos); - stringBuilder.append(renderGithubButtons()); - stringBuilder.append("
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n"); - } +// String edgeInfos = renderEdgeToRemoveInfos(edgeToRemoveInfos); +// +// if (!edgeToRemoveInfos.isEmpty()) { +// stringBuilder.append(edgeInfos); +// stringBuilder.append(renderGithubButtons()); +// stringBuilder.append("
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n"); +// } if (!rankedGodClassDisharmonies.isEmpty()) { final String[] godClassTableHeadings = @@ -313,10 +312,6 @@ private String renderEdgeToRemoveInfos(List edges) { .append("Current Cycle Count: ") .append(dsm.getCycles().size()) .append("
\n"); - stringBuilder - .append("Current Average Cycle Node Count: ") - .append(dsm.getAverageCycleNodeCount()) - .append("
\n"); stringBuilder .append("Current Total Back Edge Count: ") .append(dsm.getEdgesAboveDiagonal().size()) @@ -424,22 +419,17 @@ private String[] getEdgesToRemoveInfoTableHeadings() { return new String[] { "Edge", "Edge Weight", - "In # of Cycles", "New Cycle Count", - "New Avg Cycle Node Count", "Avg Node Δ ÷ Effort" }; } private String[] getEdgeToRemoveInfos(EdgeToRemoveInfo edgeToRemoveInfo) { return new String[] { - // "Edge", "Edge Weight", "In # of Cycles", "New Cycle Count", "New Avg Cycle Node Count", "Avg Node Count / - // Effort" + // "Edge", "Edge Weight", "In # of Cycles", "New Back Edge Count", "New Back Edge Weight Sum", "Payoff" renderEdge(edgeToRemoveInfo.getEdge()), - String.valueOf((int) edgeToRemoveInfo.getEdgeWeight()), - String.valueOf(edgeToRemoveInfo.getEdgeInCycleCount()), + String.valueOf(edgeToRemoveInfo.getRemovedEdgeWeight()), String.valueOf(edgeToRemoveInfo.getNewCycleCount()), - String.valueOf(edgeToRemoveInfo.getAverageCycleNodeCount()), String.valueOf(edgeToRemoveInfo.getPayoff()) }; } @@ -450,8 +440,7 @@ private String[] getRankedCycleSummaryData(RankedCycle rankedCycle, StringBuilde getClassName(rankedCycle.getCycleName()), rankedCycle.getPriority().toString(), String.valueOf(rankedCycle.getCycleNodes().size()), - String.valueOf(rankedCycle.getEdgeSet().size()) // , - // edgesToCut.toString() + String.valueOf(rankedCycle.getEdgeSet().size()) }; } diff --git a/spring-petclinic-rest-report.html b/spring-petclinic-rest-report.html index 9458a96f..b390f855 100644 --- a/spring-petclinic-rest-report.html +++ b/spring-petclinic-rest-report.html @@ -1,274 +1,271 @@ Refactor First Report for spring-petclinic-rest 3.4.3 - - - - - - - - - - + + + + + + +
-