Skip to content

Commit 4adf753

Browse files
author
Ben Romberg
committed
close #2: improve package cycle display
1 parent a3b6391 commit 4adf753

File tree

4 files changed

+121
-94
lines changed

4 files changed

+121
-94
lines changed

‎src/main/java/de/andrena/tools/nopackagecycles/CollectionOutput.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,29 @@
77

88
public class CollectionOutput {
99

10-
public static <T> String joinCollection(Collection<T> collection, final StringProvider<T> stringProvider) {
10+
public static <T> String joinCollection(Collection<T> collection, final StringProvider<T> stringProvider,
11+
String separator) {
1112
final StringBuilder output = new StringBuilder();
1213
joinCollection(collection, output, new Appender<T>() {
1314
public void append(T value) {
1415
output.append(stringProvider.provide(value));
1516
}
16-
});
17+
}, separator);
1718
return output.toString();
1819
}
1920

20-
public static <T> String joinArray(T[] array, StringProvider<T> stringProvider) {
21+
public static <T> String joinArray(T[] array, StringProvider<T> stringProvider, String separator) {
2122
@SuppressWarnings("unchecked")
2223
List<T> list = Arrays.asList(array);
23-
return joinCollection(list, stringProvider);
24+
return joinCollection(list, stringProvider, separator);
2425
}
2526

26-
public static <T> void joinCollection(Collection<T> collection, StringBuilder output, Appender<T> appender) {
27+
public static <T> void joinCollection(Collection<T> collection, StringBuilder output, Appender<T> appender,
28+
String separator) {
2729
boolean first = true;
2830
for (T element : collection) {
2931
if (!first) {
30-
output.append(", ");
32+
output.append(separator);
3133
}
3234
appender.append(element);
3335
first = false;

‎src/main/java/de/andrena/tools/nopackagecycles/PackageCycleOutput.java‎

Lines changed: 63 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import jdepend.framework.JavaClass;
1414
import jdepend.framework.JavaPackage;
1515
import de.andrena.tools.nopackagecycles.CollectionOutput.Appender;
16+
import de.andrena.tools.nopackagecycles.CollectionOutput.StringProvider;
1617

1718
public class PackageCycleOutput {
1819

@@ -28,90 +29,102 @@ public String getOutput() {
2829
orderPackagesByName();
2930
while (!packages.isEmpty()) {
3031
JavaPackage javaPackage = packages.get(0);
31-
packages.remove(0);
32+
packages.remove(javaPackage);
3233
if (javaPackage.containsCycle()) {
33-
appendOutputForPackage(javaPackage);
34+
appendOutputForPackageCycle(javaPackage);
3435
}
3536
}
3637
return output.toString();
3738
}
3839

40+
private void appendOutputForPackageCycle(JavaPackage javaPackage) {
41+
List<JavaPackage> cyclicPackages = getCyclicPackages(javaPackage);
42+
packages.removeAll(cyclicPackages);
43+
appendHeaderForPackageCycle(cyclicPackages);
44+
for (JavaPackage cyclicPackage : cyclicPackages) {
45+
appendOutputForPackage(cyclicPackage, cyclicPackages);
46+
}
47+
}
48+
49+
private void appendHeaderForPackageCycle(List<JavaPackage> cyclicPackages) {
50+
output.append("\n\n").append("Package-cycle found involving ");
51+
output.append(joinCollection(cyclicPackages, new StringProvider<JavaPackage>() {
52+
53+
public String provide(JavaPackage javaPackage) {
54+
return javaPackage.getName();
55+
}
56+
}, ", "));
57+
output.append(':');
58+
}
59+
3960
private void orderPackagesByName() {
4061
List<JavaPackage> packageList = new ArrayList<JavaPackage>(packages);
4162
orderByPackageName(packageList);
4263
packages = packageList;
4364
}
4465

45-
private void appendOutputForPackage(final JavaPackage javaPackage) {
46-
output.append("\n").append(javaPackage.getName()).append(" has cyclic dependency to: ");
47-
joinCollection(getAndPrependCyclicPackages(javaPackage), output, new Appender<JavaPackage>() {
48-
public void append(JavaPackage cyclicPackage) {
49-
appendOutputForCyclicPackage(javaPackage, cyclicPackage);
66+
private void appendOutputForPackage(final JavaPackage javaPackage, List<JavaPackage> cyclicPackages) {
67+
output.append("\n ").append(javaPackage.getName()).append(" depends on:");
68+
for (JavaPackage cyclicPackage : cyclicPackages) {
69+
appendOutputForCyclicPackage(javaPackage, cyclicPackage);
70+
}
71+
}
72+
73+
private void appendOutputForCyclicPackage(final JavaPackage javaPackage, JavaPackage cyclicPackage) {
74+
if (javaPackage.equals(cyclicPackage)) {
75+
return;
76+
}
77+
List<JavaClass> dependentClasses = getOrderedDependentClasses(javaPackage, cyclicPackage);
78+
if (!dependentClasses.isEmpty()) {
79+
appendOutputForDependentCyclicPackage(javaPackage, cyclicPackage, dependentClasses);
80+
}
81+
}
82+
83+
private List<JavaClass> getOrderedDependentClasses(JavaPackage javaPackage, JavaPackage cyclicPackage) {
84+
List<JavaClass> dependentClasses = new ArrayList<JavaClass>();
85+
@SuppressWarnings("unchecked")
86+
Collection<JavaClass> allClasses = javaPackage.getClasses();
87+
for (JavaClass javaClass : allClasses) {
88+
if (javaClass.getImportedPackages().contains(cyclicPackage)) {
89+
dependentClasses.add(javaClass);
5090
}
51-
});
91+
}
92+
orderByClassName(dependentClasses);
93+
return dependentClasses;
5294
}
5395

54-
private void appendOutputForCyclicPackage(JavaPackage javaPackage, JavaPackage cyclicPackage) {
55-
output.append(cyclicPackage.getName()).append(" (");
56-
appendOutputForCyclicPackageClasses(javaPackage, cyclicPackage);
96+
private void appendOutputForDependentCyclicPackage(JavaPackage javaPackage, JavaPackage cyclicPackage,
97+
List<JavaClass> dependentClasses) {
98+
output.append("\n ").append(cyclicPackage.getName()).append(" (");
99+
appendOutputForCyclicPackageClasses(javaPackage, cyclicPackage, dependentClasses);
57100
output.append(")");
58101
}
59102

60-
private void appendOutputForCyclicPackageClasses(JavaPackage javaPackage, final JavaPackage cyclicPackage) {
61-
joinCollection(getOrderedPackageClasses(javaPackage, cyclicPackage), output, new Appender<JavaClass>() {
103+
private void appendOutputForCyclicPackageClasses(JavaPackage javaPackage, final JavaPackage cyclicPackage,
104+
List<JavaClass> dependentClasses) {
105+
joinCollection(dependentClasses, output, new Appender<JavaClass>() {
62106
public void append(JavaClass packageClass) {
63107
output.append(packageClass.getName().substring(cyclicPackage.getName().length() + 1));
64108
}
65-
});
109+
}, ", ");
66110
}
67111

68-
private Collection<JavaClass> getOrderedPackageClasses(JavaPackage javaPackage, JavaPackage cyclicPackage) {
69-
List<JavaClass> classes = getClassesDependentOnPackage(javaPackage, cyclicPackage);
70-
Collections.sort(classes, new Comparator<JavaClass>() {
112+
private void orderByClassName(List<JavaClass> dependentClasses) {
113+
Collections.sort(dependentClasses, new Comparator<JavaClass>() {
71114
public int compare(JavaClass class1, JavaClass class2) {
72115
return class1.getName().compareTo(class2.getName());
73116
}
74117
});
75-
return classes;
76-
}
77-
78-
private List<JavaClass> getClassesDependentOnPackage(JavaPackage javaPackage, JavaPackage cyclicPackage) {
79-
@SuppressWarnings("unchecked")
80-
Collection<JavaClass> allClasses = cyclicPackage.getClasses();
81-
List<JavaClass> dependentClasses = new ArrayList<JavaClass>();
82-
for (JavaClass clazz : allClasses) {
83-
if (clazz.getImportedPackages().contains(javaPackage)) {
84-
dependentClasses.add(clazz);
85-
}
86-
}
87-
return dependentClasses;
88118
}
89119

90-
private List<JavaPackage> getAndPrependCyclicPackages(JavaPackage javaPackage) {
120+
private List<JavaPackage> getCyclicPackages(JavaPackage javaPackage) {
91121
List<JavaPackage> cyclicPackages = new ArrayList<JavaPackage>();
92122
javaPackage.collectAllCycles(cyclicPackages);
93-
removeSelfAndDuplications(javaPackage, cyclicPackages);
123+
removeDuplications(javaPackage, cyclicPackages);
94124
orderByPackageName(cyclicPackages);
95-
prependPackages(cyclicPackages);
96125
return cyclicPackages;
97126
}
98127

99-
private void prependPackages(List<JavaPackage> cyclicPackages) {
100-
List<JavaPackage> pendingCyclicPackages = getPendingCyclicPackages(cyclicPackages);
101-
packages.removeAll(pendingCyclicPackages);
102-
packages.addAll(0, pendingCyclicPackages);
103-
}
104-
105-
private List<JavaPackage> getPendingCyclicPackages(List<JavaPackage> cyclicPackages) {
106-
List<JavaPackage> pendingCyclicPackages = new ArrayList<JavaPackage>();
107-
for (JavaPackage cyclicPackage : cyclicPackages) {
108-
if (packages.contains(cyclicPackage)) {
109-
pendingCyclicPackages.add(cyclicPackage);
110-
}
111-
}
112-
return pendingCyclicPackages;
113-
}
114-
115128
private void orderByPackageName(List<JavaPackage> cyclicPackages) {
116129
Collections.sort(cyclicPackages, new Comparator<JavaPackage>() {
117130
public int compare(JavaPackage package1, JavaPackage package2) {
@@ -120,9 +133,8 @@ public int compare(JavaPackage package1, JavaPackage package2) {
120133
});
121134
}
122135

123-
private void removeSelfAndDuplications(JavaPackage javaPackage, List<JavaPackage> cyclicPackages) {
136+
private void removeDuplications(JavaPackage javaPackage, List<JavaPackage> cyclicPackages) {
124137
Set<JavaPackage> uniquePackages = new HashSet<JavaPackage>(cyclicPackages);
125-
uniquePackages.remove(javaPackage);
126138
cyclicPackages.clear();
127139
cyclicPackages.addAll(uniquePackages);
128140
}

‎src/test/java/de/andrena/tools/nopackagecycles/CollectionOutputTest.java‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
public class CollectionOutputTest {
1919
private final DummyAppender appender = new DummyAppender();
2020
private static final DummyStringProvider STRING_PROVIDER = new DummyStringProvider();
21+
private static final String SEPARATOR = ", ";
2122

2223
private class DummyAppender implements Appender<String> {
2324
public void append(String value) {
@@ -40,50 +41,56 @@ public void setUp() {
4041

4142
@Test
4243
public void joinCollection_WithStringProvider_WithEmptyCollection() throws Exception {
43-
assertThat(joinCollection(Collections.<String> emptyList(), STRING_PROVIDER), is(""));
44+
assertThat(joinCollection(Collections.<String> emptyList(), STRING_PROVIDER, SEPARATOR), is(""));
4445
}
4546

4647
@Test
4748
public void joinCollection_WithStringProvider_WithSingletonCollection() throws Exception {
48-
assertThat(joinCollection(singletonList("entry1"), STRING_PROVIDER), is("entry1"));
49+
assertThat(joinCollection(singletonList("entry1"), STRING_PROVIDER, SEPARATOR), is("entry1"));
4950
}
5051

5152
@Test
5253
public void joinCollection_WithStringProvider_WithCollectionHavingMultipleEntries() throws Exception {
53-
assertThat(joinCollection(asList("entry1", "entry2"), STRING_PROVIDER), is("entry1, entry2"));
54+
assertThat(joinCollection(asList("entry1", "entry2"), STRING_PROVIDER, SEPARATOR), is("entry1, entry2"));
5455
}
5556

5657
@Test
5758
public void joinArray_WithStringProvider_WithEmptyArray() throws Exception {
58-
assertThat(joinArray(new String[0], STRING_PROVIDER), is(""));
59+
assertThat(joinArray(new String[0], STRING_PROVIDER, SEPARATOR), is(""));
5960
}
6061

6162
@Test
6263
public void joinArray_WithStringProvider_WithSingletonArray() throws Exception {
63-
assertThat(joinArray(new String[] { "entry1" }, STRING_PROVIDER), is("entry1"));
64+
assertThat(joinArray(new String[] { "entry1" }, STRING_PROVIDER, SEPARATOR), is("entry1"));
6465
}
6566

6667
@Test
6768
public void joinArray_WithStringProvider_WithArrayHavingMultipleEntries() throws Exception {
68-
assertThat(joinArray(new String[] { "entry1", "entry2" }, STRING_PROVIDER), is("entry1, entry2"));
69+
assertThat(joinArray(new String[] { "entry1", "entry2" }, STRING_PROVIDER, SEPARATOR), is("entry1, entry2"));
6970
}
7071

7172
@Test
7273
public void joinCollection_WithAppender_WithEmptyCollection() throws Exception {
73-
joinCollection(Collections.<String> emptyList(), output, appender);
74+
joinCollection(Collections.<String> emptyList(), output, appender, SEPARATOR);
7475
assertThat(output.toString(), is(""));
7576
}
7677

7778
@Test
7879
public void joinCollection_WithAppender_WithSingletonCollection() throws Exception {
79-
joinCollection(singletonList("entry1"), output, appender);
80+
joinCollection(singletonList("entry1"), output, appender, SEPARATOR);
8081
assertThat(output.toString(), is("entry1"));
8182
}
8283

8384
@Test
8485
public void joinCollection_WithAppender_WithCollectionHavingMultipleEntries() throws Exception {
85-
joinCollection(asList("entry1", "entry2"), output, appender);
86+
joinCollection(asList("entry1", "entry2"), output, appender, SEPARATOR);
8687
assertThat(output.toString(), is("entry1, entry2"));
8788
}
8889

90+
@Test
91+
public void joinCollection_WithAppender_WithNewlineSeparator() throws Exception {
92+
joinCollection(asList("entry1", "entry2"), output, appender, "\n");
93+
assertThat(output.toString(), is("entry1\nentry2"));
94+
}
95+
8996
}

0 commit comments

Comments
 (0)