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

Recursive, nested list traversal

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

Problem

Given a nested list of strings (which may contain other nested lists), print the contents of the list and the corresponding depth.

Here is my solution:

private static void dumpList(String string, List l) {
    int n = l.size();
    for (int i = 0; i < n; i++) {
        if(l.get(i) instanceof String){
            System.out.println(string+i + " " + l.get(i));
        }
        if(l.get(i) instanceof List){
            dumpList(string+i, (List)l.get(i));
        }
    }
}


Here is main():

public static void main(String[] args) {
    String s = "Foo";
    List l = new ArrayList();
    ArrayList a = new ArrayList();
    ArrayList b = new ArrayList();
    //ArrayList c = new ArrayList();
    a.add("a");
    a.add("b");
    a.add("c");
    b.add("eggs");
    l.add("a string");
    l.add(a);
    l.add("spam");
    l.add("eggs");
    dumpList("Foo", l);
}


My output:

Foo0 a string
Foo10 a
Foo11 b
Foo12 c
Foo2 spam
Foo3 eggs


I am not that much interested in naming of the variables, but of formatting of the output. Should I use StringBuilder instead of the + operator? Should I print it differently?

Solution

This is in addition to what @tim already said.

The dumpList method takes a List,
and then you make 2 .get(i) calls in every iteration of the loop.
Keep in mind that the List interface doesn't ensure random access.
For example I can call this method with a LinkedList parameter,
in which case the .get(i) calls will be costly.
Either change the method parameter to ArrayList or similar random access list,
or change the way you iterate (I recommend), for example like this:

private static void dumpList(String string, List list) {
    int i = 0;
    for (Object item : list) {
        if (item instanceof List) {
            dumpList(string + i, (List) item);
        } else {
            System.out.println(String.format("%s%d %s", string, i, item));
        }
        ++i;
    }
}


Instead of doing if (item instanceof String) and then if (item instanceof List),
it's better to do if (item instanceof List) and an else.
This solves 2 problems at once:

-
Adds support for other element types, not only String and List,
which otherwise would have been ignored

-
The second if should have been either an else if or an else,
because if the first if was true, it's pointless to evaluate the second

I changed the string + i + " " + l.get(i) to the (in my opinion) more readable String.format("...", ...) style.

Finally, I renamed l to list.
Single-letter variable names are acceptable as loop counters, like i, j, k,
otherwise it's better to give meaningful names.

Code Snippets

private static void dumpList(String string, List<Object> list) {
    int i = 0;
    for (Object item : list) {
        if (item instanceof List) {
            dumpList(string + i, (List) item);
        } else {
            System.out.println(String.format("%s%d %s", string, i, item));
        }
        ++i;
    }
}

Context

StackExchange Code Review Q#60320, answer score: 8

Revisions (0)

No revisions yet.