8
\$\begingroup\$

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);
    }
}
Improve this question
\$\endgroup\$
3
  • \$\begingroup\$ entries and moreToCome 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\$
    – Marc-Andre
    Apr 2, 2014 at 12:48
  • \$\begingroup\$ That's right. It's not that global variable what you think. But on the other way I thought about passing these variables as parameters to the methods I have. \$\endgroup\$
    – bobbel
    Apr 2, 2014 at 13:11
  • \$\begingroup\$ I would not add it as a parameter since it would add complexity to the method calling. There is nothing wrong with having private class variable as long as their scope is good (which look like it). \$\endgroup\$
    – Marc-Andre
    Apr 2, 2014 at 13:17

2 Answers 2

Reset to default
5
\$\begingroup\$

Just two quick points (for the original code):

  1. 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.)

  2. I'd try to add a boolean parameter (lastElementInThisLevel or something similar) to the printHierarchy method to remove the moreToCome 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 and e.)

Improve this answer
\$\endgroup\$
1
  • \$\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
    Apr 2, 2014 at 14:53
4
\$\begingroup\$

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
}
Improve this answer
\$\endgroup\$
4
  • \$\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 a Map<Integer, Boolean> or just a List<Boolean>? If you understand what it should do, what's your preferred way of solving this? \$\endgroup\$
    – bobbel
    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
    Apr 2, 2014 at 14:36
  • \$\begingroup\$ @bobbel - I've updated the answer according to your changes \$\endgroup\$
    – Uri Agassi
    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
    Apr 2, 2014 at 20:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.