2
\$\begingroup\$

I'm currently working on an uncrafting tool for minecraft and got a neat output working that shows all the materials needed in a tree. I wonder if the code used to print that tree could be improved somehow. I only have the recursion depth as a measurement and the ArrayList gets filled in the same order as seen in the input example below. The output format should also stay the same, I'm quite happy with that. The input format is a bit sketchy though, any ideas on that are appreciated as well.

The Algorithm

 private static void treeprint(ArrayList<Object[]> tree) { ArrayList<String> stem = new ArrayList<>(); for (int i = 0; i < tree.size() - 1; i++) { System.out.print(String.join("", stem)); Object[] o = tree.get(i); int cdepth = (int) o[0]; String cname = (String) o[1]; int camt = (int) o[2]; int ndepth = (int) tree.get(i + 1)[0]; if (ndepth > cdepth) { if (isLastOfDepth(tree, cdepth, i + 1)) { System.out.print("\\---"); stem.add(" "); } else { System.out.print("+---"); stem.add("| "); } } if (ndepth == cdepth) { if (isLastOfDepth(tree, cdepth, i + 1)) { System.out.print("\\---"); } else { System.out.print("+---"); } } if (ndepth < cdepth) { System.out.print("\\---"); for (int j = 0; j < cdepth - ndepth; j++) { stem.remove(stem.size() - 1); } } System.out.println(cname + " x " + camt); } System.out.println(String.join("", stem) + "\\---" + (String) tree.get(tree.size() - 1)[1] + " x " + (int) tree.get(tree.size() - 1)[2]); } private static boolean isLastOfDepth(ArrayList<Object[]> tree, int depth, int start) { for (int i = start; i < tree.size(); i++) { int cdepth = (int) tree.get(i)[0]; if (cdepth < depth) { return true; } if (cdepth == depth) { return false; } } return true; } 

Test Input

Format is recuresion depth / item name / item amount

1 component heat vent 1 2 iron bars 4 3 iron 6 2 heat vent 1 3 electric motor 1 4 coil 2 5 copper cable 8 5 iron 1 4 iron 1 4 tin item casing 2 3 iron bars 4 4 iron 6 3 iron plate 4 2 tin plate 4 
\$\endgroup\$

    1 Answer 1

    2
    \$\begingroup\$

    Nice solution, my suggestions:

    • Short variable names: variables like camt and cname can be extended to currentAmount and currentName. It's better to have longer names to improve readability. For example, I am not sure what stem stands for.

    • Types and input validation: there are a lot of casting in the methods which are not validated. Accepting an array of Object without any validation is a recipe for troubles. Using a class you can solve both problems, something like this:

      class Node{ private int depth; private String itemName; private int amount; public Node(int depth, String itemName, int amount) { // Input validation here this.depth = depth; this.itemName = itemName; this.amount = amount; } // Getters, etc. } 

      So this code:

      Object[] o = tree.get(i); int cdepth = (int) o[0]; String cname = (String) o[1]; int camt = (int) o[2]; 

      Can become:

      Node node = tree.get(i); int currentDepth = node.getDepth(); String currentItemName = node.getItemName(); int currentAmount = node.getAmount(); 

      You can also make the properties of the class final to have an immutable node.

    • Constants: the strings \--- and +--- are repeated multiple times. Consider creating constants for such special strings so that they will be easier to change in the future.

    • Functionality: If the list is not properly built, the tree will not be printed correctly. For example: The input:

      1 a 1 2 b 1 3 c 1 4 d 1 

      Produces the correct output:

      \---a x 1 \---b x 1 \---c x 1 \---d x 1 

      But if we add one more node in the middle:

      1 a 1 2 b 1 3 c 1 2 e 1 4 d 1 

      The output becomes:

      \---a x 1 +---b x 1 | \---c x 1 \---e x 1 \---d x 1 

      Now d changed its depth from 4 to 3 and e became its father.

      Providing a well-formed list of nodes with corresponding depth is not a simple task if the tree grows in size. A solution might be to create a Tree class that knows how to add and remove nodes. Then the treePrint method will do a Pre-order scan of the tree and print the nodes on the way.

    \$\endgroup\$
    1
    • 1
      \$\begingroup\$Thanks! Var names and constants do need a refactoring. As for the Object[]: The entries are either internal counters or validated elsewhere. Still a valid point, perhaps a class would still be better for readability. As for functionality, this is a bug but one that shouldn't occur. The ArrayList is generated in order as the tree of recipe dependencies is explored depth-first via recursion. This way, the cdepth can only grow by 1 (but shrink to any number >0).\$\endgroup\$CommentedFeb 18, 2021 at 14:24

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.