Problem
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:
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.
Solution
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
forNode
(andtestCreateTree()
shows one is urgently needed)
(what use is parent of aNode
?)
(come to think of it:
what use are all the setters?
getNodeType()
could usenull == children
, and parentnull
or not) - one ticklish part in designing
Node
is deciding whether construction of aNode
withnull != parent
should add the newNode
toparent
‘s children.
(One alternative being an instance methodNode 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 usingIterable.forEach()
(which seems to preclude “early out” – ?):
if onlyjava.util.Collection
(or, at least,Set
) provided for anE find(Object toEqual)
: implementequals()
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 aString
context” (e.g.,+
multipleString
s). Doing so allowsNullPointerException
s (which are sidestepped (implicitly) usingString.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 byprintNode()
– that string is formatted horribly, anyway)
- use a
- for all I don’t know about JSON, I’d pattern support after
toString()
or (de/)serialisation. -
printNodeJSON()
is weird for duplicatingprintNodeJSONWithoutChildren()
‘s code instead of using it.
Neither prints. -
in
testCreateTree()
, you walk theNode
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 - Why provide parameters to