HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Java Recursive Depth First Search

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
depthsearchjavarecursivefirst

Problem

I've created a recursive depth-first search implementation in Java as an example for an article I am writing on my website. It needs to be concise in order to fit on the page easily (independent of screen sizes), hence the lack of extra spacing.

public static boolean depthFirstSearch(TreeNode find, TreeNode top){
   if(top == null || find == null) return false;
   else if(top.equals(find)) return true;
   Enumeration children = top.children();
   while(children.hasMoreElements()){
       TreeNode next = (TreeNode) children.nextElement();
       if(depthFirstSearch(find, next)){
           return true;
       }
   }
   return false;
}


I am using javax.swing.tree.TreeNode so that I don't have to include any custom node implementations and the search is only looking for the presence of a node, not position.

Solution

The context of this code example is not very clear, so in my remarks below I will suppose that the code is about a generic implementation of depth-first search.

TreeNode

If this is a generic implementation, my first question then is why TreeNode interface is there? This interface is part of Swing API, which has it own sphere of application. The objects implementing this interface may (or may not) be convenient here; anyway, they tend to be used in Swing/GUI-related situations and I'd not interfere with their scope for a generic case.

There is also something even more important concerning TreeNode. As you know, it has children() method which returns an Enumeration. This type is a very old Java thing (since JDK1.0), introduced even before Collections, and currently it looks like a rudiment. And it's not very pleasant to iterate on.

But if you still need/want to keep TreeNode, a type bound mark Enumeration should be added, in order to avoid compilation warnings.

To replace TreeNode, I'd suggest to create a short and concise Node class, which points to a Collection (or even a Stream) of children Nodes and overrides hashCode and equals methods. This class would also be useful in other examples of your article.

Code Blocks

I understand that conciseness and lack of extra space is the priority in this code, but when I see this

if(top == null || find == null) return false;
else if(top.equals(find)) return true;


it looks for me more like code obfuscation. Please find a place to use braces:

if (top == null || find == null) {
    return false;
}
else if (top.equals(find)) {
     return true;
}


It can be very confusing and error-prone when the instruction that immediately follows a condition is not wrapped in a block.

Code Snippets

if(top == null || find == null) return false;
else if(top.equals(find)) return true;
if (top == null || find == null) {
    return false;
}
else if (top.equals(find)) {
     return true;
}

Context

StackExchange Code Review Q#105122, answer score: 4

Revisions (0)

No revisions yet.