I'm coding a little class hierarchy printing tool for easy show hierarchies of java classes.
This is my current code:
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.TreeMap;
import javax.swing.JDialog;
import jdk.nashorn.internal.runtime.PropertyMap; // since Java 8, just for testing
import com.sun.javafx.collections.TrackableObservableList;
public class PrintClassHierarchy {
private static final String PADDING = " ";
private static final String PADDING_WITH_COLUMN = " | ";
private static final String PADDING_WITH_ENTRY = " |--- ";
private static final String BASE_CLASS = Object.class.getName();
private TreeMap<String, List<String>> entries;
private boolean[] moreToCome;
public static void main(final String[] args) {
new PrintClassHierarchy().printHierarchy(
PrintStream.class,
FileOutputStream.class,
FileInputStream.class,
TrackableObservableList.class,
PropertyMap.class,
JDialog.class
);
}
public void printHierarchy(final Class<?>... clazzes) {
// clean values
entries = new TreeMap<>();
moreToCome = new boolean[99];
// get all entries of tree
traverseClasses(clazzes);
// print collected entries as ASCII tree
printHierarchy(BASE_CLASS, 0);
}
private void printHierarchy(final String node, final int level) {
for (int i = 1; i < level; i++) {
System.out.print(moreToCome[i - 1] ? PADDING_WITH_COLUMN : PADDING);
}
if (level > 0) {
System.out.print(PADDING_WITH_ENTRY);
}
System.out.println(node);
if (entries.containsKey(node)) {
final List<String> list = entries.get(node);
for (int i = 0; i < list.size(); i++) {
moreToCome[level] = (i < list.size() - 1);
printHierarchy(list.get(i), level + 1);
}
}
}
private void traverseClasses(final Class<?>... clazzes) {
Arrays.asList(clazzes).forEach(c -> traverseClasses(c, 0));
}
private void traverseClasses(final Class<?> clazz, final int level) {
final Class<?> superClazz = clazz.getSuperclass();
if (superClazz == null) {
return;
}
final String name = clazz.getName();
final String superName = superClazz.getName();
if (entries.containsKey(superName)) {
final List<String> list = entries.get(superName);
if (!list.contains(name)) {
list.add(name);
Collections.sort(list); // SortedList
}
} else {
entries.put(superName, new ArrayList<String>(Arrays.asList(name)));
}
traverseClasses(superClazz, level + 1);
}
}
This prints out:
java.lang.Object |--- java.awt.Component | |--- java.awt.Container | |--- java.awt.Window | |--- java.awt.Dialog | |--- javax.swing.JDialog |--- java.io.InputStream | |--- java.io.FileInputStream |--- java.io.OutputStream | |--- java.io.FileOutputStream | |--- java.io.FilterOutputStream | |--- java.io.PrintStream |--- java.util.AbstractCollection | |--- java.util.AbstractList | |--- javafx.collections.ObservableListBase | |--- javafx.collections.ModifiableObservableListBase | |--- com.sun.javafx.collections.ObservableListWrapper | |--- com.sun.javafx.collections.TrackableObservableList |--- jdk.nashorn.internal.runtime.PropertyMap
This output is already valid. I want to know, if this is a good approach to do the task, or if there is another (better) way to do this?
I'm not sure, if the entries
and moreToCome
"global" variables are the way to go.
Or if you have general improvements, please let me know!
Edit 1:
I reworked the code to find out the maximum level in hierarchy to discover how big the boolean array has to be. Also I reworked the method calls and put the traverseClasses
into the constructor as recommended. Furthermore I renamed some variables to more descriptive names and added some comments. moreToCome
is now moreClassesInHierarchy
.
Second approach:
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.TreeMap;
import javax.swing.JDialog;
import jdk.nashorn.internal.runtime.PropertyMap; // since Java 8, just for testing
import com.sun.javafx.collections.TrackableObservableList;
public class PrintClassHierarchy {
private static final String PADDING = " ";
private static final String PADDING_WITH_COLUMN = " | ";
private static final String PADDING_WITH_ENTRY = " |--- ";
private static final String BASE_CLASS = Object.class.getName();
private final TreeMap<String, List<String>> subClazzEntries;
private final boolean[] moreClassesInHierarchy;
private int maxLevel = 0;
public static void main(final String[] args) {
new PrintClassHierarchy(
PrintStream.class,
FileOutputStream.class,
FileInputStream.class,
TrackableObservableList.class,
PropertyMap.class,
JDialog.class
).printHierarchy();
}
public PrintClassHierarchy(final Class<?>... clazzes) {
subClazzEntries = new TreeMap<>();
// get all entries of tree
traverseClasses(clazzes);
// initialize array with size of maximum class hierarchy level
moreClassesInHierarchy = new boolean[maxLevel];
}
public void printHierarchy() {
// print collected entries as ASCII tree
printHierarchy(BASE_CLASS, 0);
}
private void printHierarchy(final String clazzName, final int level) {
for (int i = 1; i < level; i++) {
// The flag moreToCome holds an identifier, if there is another class
// on the specific value, that comes beneath the current class.
// So print either '|' or ' '.
System.out.print(moreClassesInHierarchy[i - 1] ? PADDING_WITH_COLUMN : PADDING);
}
if (level > 0) {
System.out.print(PADDING_WITH_ENTRY);
}
System.out.println(clazzName);
if (subClazzEntries.containsKey(clazzName)) {
final List<String> list = subClazzEntries.get(clazzName);
for (int i = 0; i < list.size(); i++) {
// if there is another class that comes beneath the next class,
// flag this level
moreClassesInHierarchy[level] = (i < list.size() - 1);
printHierarchy(list.get(i), level + 1);
}
}
}
private void traverseClasses(final Class<?>... clazzes) {
// do the traverseClasses on each provided class (possible since Java 8)
Arrays.asList(clazzes).forEach(c -> traverseClasses(c, 0));
}
private void traverseClasses(final Class<?> clazz, final int level) {
final Class<?> superClazz = clazz.getSuperclass();
if (level > maxLevel) {
// discover maximum level
maxLevel = level;
}
if (superClazz == null) {
// we arrived java.lang.Object
return;
}
final String name = clazz.getName();
final String superName = superClazz.getName();
if (subClazzEntries.containsKey(superName)) {
final List<String> list = subClazzEntries.get(superName);
if (!list.contains(name)) {
list.add(name);
Collections.sort(list); // SortedList
}
} else {
subClazzEntries.put(superName, new ArrayList<String>(Arrays.asList(name)));
}
traverseClasses(superClazz, level + 1);
}
}
Edit 2:
Now I implemented the Stack
solution from Uri Agassi in my code.
This is much more cleaner, since I don't have to care about the Stack size and the current level. It just fits the requirement!
Furthermore I got rid of the TreeMap
since I don't really need a sorted map. Therefore Map
(HashMap
) remains.
Third approach:
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Stack;
import javax.swing.JDialog;
import jdk.nashorn.internal.runtime.PropertyMap; // since Java 8, just for testing
import com.sun.javafx.collections.TrackableObservableList;
public class PrintClassHierarchy {
private static final String PADDING = " ";
private static final String PADDING_WITH_COLUMN = " | ";
private static final String PADDING_WITH_ENTRY = " |--- ";
private static final String BASE_CLASS = Object.class.getName();
private final Map<String, List<String>> subClazzEntries = new HashMap<>();
public static void main(final String[] args) {
new PrintClassHierarchy(
PrintStream.class,
FileOutputStream.class,
FileInputStream.class,
TrackableObservableList.class,
PropertyMap.class,
JDialog.class
).printHierarchy();
}
public PrintClassHierarchy(final Class<?>... clazzes) {
// get all entries of tree
traverseClasses(clazzes);
}
public void printHierarchy() {
// print collected entries as ASCII tree
printHierarchy(BASE_CLASS, new Stack<Boolean>());
}
private void printHierarchy(final String clazzName, final Stack<Boolean> moreClassesInHierarchy) {
if (!moreClassesInHierarchy.empty()) {
for (final Boolean hasColumn : moreClassesInHierarchy.subList(0, moreClassesInHierarchy.size() - 1)) {
System.out.print(hasColumn.booleanValue() ? PADDING_WITH_COLUMN : PADDING);
}
}
if (!moreClassesInHierarchy.empty()) {
System.out.print(PADDING_WITH_ENTRY);
}
System.out.println(clazzName);
if (subClazzEntries.containsKey(clazzName)) {
final List<String> list = subClazzEntries.get(clazzName);
for (int i = 0; i < list.size(); i++) {
// if there is another class that comes beneath the next class, flag this level
moreClassesInHierarchy.push(new Boolean(i < list.size() - 1));
printHierarchy(list.get(i), moreClassesInHierarchy);
moreClassesInHierarchy.removeElementAt(moreClassesInHierarchy.size() - 1);
}
}
}
private void traverseClasses(final Class<?>... clazzes) {
// do the traverseClasses on each provided class (possible since Java 8)
Arrays.asList(clazzes).forEach(c -> traverseClasses(c, 0));
}
private void traverseClasses(final Class<?> clazz, final int level) {
final Class<?> superClazz = clazz.getSuperclass();
if (superClazz == null) {
// we arrived java.lang.Object
return;
}
final String name = clazz.getName();
final String superName = superClazz.getName();
if (subClazzEntries.containsKey(superName)) {
final List<String> list = subClazzEntries.get(superName);
if (!list.contains(name)) {
list.add(name);
Collections.sort(list); // SortedList
}
} else {
subClazzEntries.put(superName, new ArrayList<String>(Arrays.asList(name)));
}
traverseClasses(superClazz, level + 1);
}
}
2 Answers 2
Just two quick points (for the original code):
You could save a lot of logic with a
TreeMultimap
(from Google Guava):private final TreeMultimap<String, String> entries = TreeMultimap.create();
This:
if (entries.containsKey(node)) { final List<String> list = entries.get(node); for (int i = 0; i < list.size(); i++) { moreToCome[level] = (i < list.size() - 1); printHierarchy(list.get(i), level + 1); } }
could be replaced with
for (String e: list) { moreToCome[level] = (i < list.size() - 1); printHierarchy(e, level + 1); i++; }
and this:
if (entries.containsKey(superName)) { final List<String> list = entries.get(superName); if (!list.contains(name)) { list.add(name); Collections.sort(list); // SortedList } } else { entries.put(superName, new ArrayList<String>(Arrays.asList(name))); }
could be replaced with
entries.put(superName, name);
(See also: Effective Java, 2nd edition, Item 47: Know and use the libraries The author mentions only the JDK's built-in libraries but I think the reasoning could be true for other libraries too.)
I'd try to add a boolean parameter (
lastElementInThisLevel
or something similar) to theprintHierarchy
method to remove themoreToCome
array and clean this loop:int i = 0; for (String e: list) { moreToCome[level] = (i < list.size() - 1); printHierarchy(e, level + 1); i++; }
(Use more descriptive names than
list
ande
.)
-
\$\begingroup\$ Thank you for your recommendations! I'm not using Guava yet, because I like to implement things without external libraries. My first thought about using external libraries is, that they will blow up the size of your application. For example, my program will be a few KB. With the library it will be over 2 MB! Finally, the library is perfect for doing some sub tasks of my problem. Thank you very much for showing this up! \$\endgroup\$bobbel– bobbel2014年04月02日 14:53:41 +00:00Commented Apr 2, 2014 at 14:53
Mutators vs. Selectors
The method printHierarchy(final Class<?>...)
both populates the instance, and prints the result. It would be a lot more predictable if it would be responsible only for one or the other. Its name implies that it only prints, so I would suggest to move the population of the PrintClassHierarchy
to its constructor, leaving only the printing to the printHierarchy
method:
public static void main(final String[] args) {
new PrintClassHierarchy().printHierarchy(
PrintStream.class,
FileOutputStream.class,
FileInputStream.class,
TrackableObservableList.class,
PropertyMap.class,
JDialog.class
).printHierarchy();
}
More to come...?
I don't have a concrete suggestion on how you should implement it, but the moreToCome
solution you have is a little dodgy:
- It is not apparent how it works
- You initialize it in an arbitrary manner (
new boolean[99]
) - always a bad sign... - Its usage looks very brittle, and any failure might be very hard to debug
More to come - part 2 (the second coming?)
After you've refactored the mutator out of your selector, it is obvious that the only state change while printing your classes is the moreClassesInHierarchy
(nee moreToCome
) member. But a selector class should not change the state of the object!
The design which emerges from this is that moreToCome
should probably be a parameter which changes before and after the recursion, probably a Stack<boolean>
:
for (int i = 0; i < list.size(); i++) {
// if there is another class that comes beneath the next class,
// flag this level
moreClassesInHierarchy.push(i < list.size() - 1);
printHierarchy(list.get(i), moreClassesInHierarchy);
moreClassesInHierarchy.pop();
}
Note that in this solution, I've gotten rid of level
, since is hidden in moreClassesInHierarchy.size()
...
So now the method becomes:
public void printHierarchy() {
// print collected entries as ASCII tree
printHierarchy(BASE_CLASS, new Stack<boolean>());
}
private void printHierarchy(final String clazzName, final Stack<boolean> moreClassesInHierarchy) {
for (boolean hasColumn : moreClassesInHierarchy) {
System.out.print(hasColumn ? PADDING_WITH_COLUMN : PADDING);
}
if (!moreClassesInHierarchy.empty()) {
System.out.print(PADDING_WITH_ENTRY);
}
// ...cont
}
-
\$\begingroup\$ Thank you for your response! You're right. The
moreToCome
thingy is not well explained. What it does: it should tell the next line, if there will be another class on the same level beneath it for printing the column|
on each level. Unfortunately, I can't explain it better... Also it's true, that initializing the array with a fixed length is a bad sign. I'm not sure how to replace it. Perhaps with aMap<Integer, Boolean>
or just aList<Boolean>
? If you understand what it should do, what's your preferred way of solving this? \$\endgroup\$bobbel– bobbel2014年04月02日 13:07:28 +00:00Commented Apr 2, 2014 at 13:07 -
\$\begingroup\$ Now I let discover the maximum level of hierarchy, so the array can be initialized with the fewest amount of fields. \$\endgroup\$bobbel– bobbel2014年04月02日 14:36:30 +00:00Commented Apr 2, 2014 at 14:36
-
\$\begingroup\$ @bobbel - I've updated the answer according to your changes \$\endgroup\$Uri Agassi– Uri Agassi2014年04月02日 16:10:54 +00:00Commented Apr 2, 2014 at 16:10
-
1\$\begingroup\$ Thanks again for the great idea of using a
Stack
! I reworked my class again. Only in your for-each loop, I had to use a sub list of all elements except the last one. Otherwise it printed out a weird result. \$\endgroup\$bobbel– bobbel2014年04月02日 20:03:39 +00:00Commented Apr 2, 2014 at 20:03
entries
andmoreToCome
are private instance variable which is looking fine to me since they look as being use in every method of the class. If they were global that would be a really big problem ;) . \$\endgroup\$