2
\$\begingroup\$

I have this list of paths:

private static final List<String> paths = Arrays.asList(
        "assets/css/custom.css",
        "assets/css/default.css",
        "assets/js/main.js",
        "assets/js/old/main-old.js",
        "fonts/poppins.woff",
        "favicon.ico",
        "index.html"
);

That I need to create a searchable tree, like this:

enter image description here

and here's what I have now:

public void testCreateTree() {
    Node root = new Node("ROOT", null, Node.NODE_TYPE.ROOT);
    paths.forEach(path -> {
        final Node[] currentNode = {root};
        if(!path.contains("/")) { // root files
            currentNode[0].addChild(new Node(path, currentNode[0], Node.NODE_TYPE.FILE));
        } else {
            String folders = DirectoryRegex.matchFolders(path); // e.g. matches/returns "root/"
            String fileName = DirectoryRegex.matchFile(path); // e.g. matches/returns index.html
            String[] folderArrays = folders.split("/");
            Arrays.asList(folderArrays).forEach(folder -> {
                Node node = new Node("ROOT", null, Node.NODE_TYPE.ROOT);
                node.setNodeName(folder);
                node.setNodeType(Node.NODE_TYPE.FOLDER);
                node.setParent(currentNode[0]);
                // check if child exists
                Node existingNode = currentNode[0].getChild(folder, Node.NODE_TYPE.FOLDER);
                if(existingNode == null) {
                    existingNode = node;
                    currentNode[0].addChild(node);
                }
                currentNode[0] = existingNode;
            });
            currentNode[0].addChild(new Node(fileName, currentNode[0], Node.NODE_TYPE.FILE));
        }

    });
    String print = root.printNodeJSON().toString();
    Console.log(print);
}

The Node.java class is this:

public class Node {
    public NODE_TYPE getNodeType() {
        return nodeType;
    }

    public void setNodeType(NODE_TYPE nodeType) {
        this.nodeType = nodeType;
    }

    public Node getParent() {
        return parent;
    }

    public void setParent(Node parent) {
        this.parent = parent;
    }

    public List<Node> getChildren() {
        if(children == null) {
            children = new LinkedList<>();
        }
        return children;
    }

    public void setChildren(List<Node> children) {
        this.children = children;
    }

    public void addChild(Node child) {
        getChildren().add(child);
    }

    public Node getChild(String nodeName, NODE_TYPE nodeType) {
        final Node[] child = {null};
        getChildren().forEach(node -> {
            if(node.getNodeName().equals(nodeName) && node.getNodeType().equals(nodeType)) {
                child[0] = node;
            }
        });
        return child[0];
    }

    public String getNodeName() {
        return nodeName;
    }

    public void setNodeName(String nodeName) {
        this.nodeName = nodeName;
    }

    private Node() {}

    public Node(String nodeName, Node parent, NODE_TYPE nodeType) {
        setNodeName(nodeName);
        setNodeType(nodeType);
        setParent(parent);
    }

    public enum NODE_TYPE { FILE, FOLDER, ROOT }
    private NODE_TYPE nodeType;
    private Node parent;
    private List<Node> children;
    private String nodeName;

    public String printNode() {
        final String[] s = {"["};

        s[0] = s[0] + "Node name: " + nodeName + ",";
        if(nodeType != null) {
            s[0] = s[0] + "Node type: " + nodeType.toString()  + ",";
        }
        if(getParent() != null) {
            s[0] = s[0] + "Node Parent: [ name = " + getParent().getNodeName() + ", type = " + getParent().getNodeType() + " ]";
        }
        s[0] = s[0] + "Node children: [";
        getChildren().forEach(child -> {
            s[0] = "[" + s[0] + child.printNode() + "]";
        });
        s[0] = s[0] + "]";

        s[0] = s[0] + "]";
        return s[0];
    }

    public JSONObject printNodeJSON() {
        JSONObject jsonObject = new JSONObject();
        jsonObject.put("nodeName", nodeName);
        jsonObject.put("nodeType", nodeType != null ? nodeType.toString() : null);
        jsonObject.put("parent", getParent() != null ? getParent().printNodeJSONWithoutChildren() : null);
        JSONArray children = new JSONArray();
        getChildren().forEach(child -> {
            children.put(child.printNodeJSON());
        });
        jsonObject.put("children", children);
        return jsonObject;
    }

    public JSONObject printNodeJSONWithoutChildren() {
        JSONObject jsonObject = new JSONObject();
        jsonObject.put("nodeName", nodeName);
        jsonObject.put("nodeType", nodeType != null ? nodeType.toString() : null);
        jsonObject.put("parent", getParent() != null ? getParent().printNodeJSONWithoutChildren() : null);
//        JSONArray children = new JSONArray();
//        getChildren().forEach(child -> {
//            children.put(child.printNodeJSON());
//        });
//        jsonObject.put("children", children);
        return jsonObject;
    }

}

The code works fine but I want to know the most efficient way to do this.

\$\endgroup\$
  • \$\begingroup\$ Would you allow paths like "./" or "temp/../other"? \$\endgroup\$ – dfhwze Jul 15 at 5:42
  • \$\begingroup\$ Is DirectoryRegex a custom class you have written? \$\endgroup\$ – dfhwze Jul 15 at 5:47
  • \$\begingroup\$ @dfhwze yes it is a custom class; and no, there will be no empty path, as every absolute path in the List ends with a file (similar to object storages like S3, where the path is actually virtual) \$\endgroup\$ – xybrek Jul 15 at 17:14
1
\$\begingroup\$

Short take …
most efficient way to do this
resources including developer time

… of uncommented code:

Starting with testCreateTree() raises hope for a stab at test first - I don't see it.

What is this above: is there more to tree than Node, how does searchable manifest?

  • there is no interface for Node
    (and testCreateTree() shows one is urgently needed)
    (what use is parent of a Node?)
    (come to think of it:
     what use are all the setters?
    getNodeType() could use null == children, and parent null or not)
  • one ticklish part in designing Node is deciding whether construction of a Node with null != parent should add the new Node to parent's children.
    (One alternative being an instance method Node addChild(name …) which instantiates a child, adds it to children and returns it.)
  • the one thing Node provides beyond data members&infrastructure is get child by name and type, which is only useful if there are to be children of identical name, differing in type, only
  • with neither assignment nor setter invocation, the lazy instantiation of children doesn't work
  • getChild(name, type) uses linear search (which may be justified if the expected number of children is quite small) - open coded, if using Iterable.forEach() (which seems to preclude "early out" - ?):
    if only java.util.Collection (or, at least, Set) provided for an E find(Object toEqual): implement equals() and use that.

    • the use of an array with one solitary element is uncalled for
  • "The Way" to support a representation of an instance for human reception is a implementing toString()
      - the use of an array with one solitary element is uncalled for

    • use a StringBuilder to build strings
    • you don't need to use .toString() "in a String context" (e.g., + multiple Strings). Doing so allows NullPointerExceptions (which are sidestepped (implicitly) using String.valueOf()).
    • there's a way to avoid <some (involved) lvalue expression> = <same (involved) lvalue expression> <operator> <expression>: compound assignment operators (e.g., <(involved) lvalue expression> /= <expression>)
      (should prevent the funny pile of '['s at the beginning of the string produced by printNode() - that string is formatted horribly, anyway)
  • for all I don't know about JSON, I'd pattern support after toString() or (de/)serialisation.
  • printNodeJSON() is weird for duplicating printNodeJSONWithoutChildren()'s code instead of using it.
    Neither prints.

  • in testCreateTree(), you walk the Node structure, potentially searching for the same names time and again.
      - the use of an array with one solitary element is uncalled for

    • Why provide parameters to Node's constructor to go on and set deviating values?

    As an alternative, separate path lookup and tree:
    get the index of the last path part separator ('/'), if any.
    if separator was found, use root as folder
    else look up the folder for the part up to that separator (path)
      if not found, create folder(s recursively) linked up with its parent and enter into path->folder lookup
    create a leaf for the part to the end "in" that folder

\$\endgroup\$
  • \$\begingroup\$ (The current fad in getting JSON objects seems to be Json.createXyzBuilder().add(…).…().build().) \$\endgroup\$ – greybeard Jul 16 at 8:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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