Question

I'm trying to figure out a way to construct a LinkedList with a for-each loop.

I've put the way I currently have it and it works. I just find it to be very ugly, since I have to declare a single listNode outside the loop and the rest inside a for loop. I just can't shake the feeling this is a clumsy solution.

How It's written-

private listNode nameList;

public listManager(ArrayList<String> names) {
    nameList = new listNode(names.get(0));
    listNode current = nameList;
    for (int i = 1;  i < names.size(); i++) {
        current.next = new listNode(names.get(i));
        current = current.next;
    }
}

I want to use a for-each loop and make everything just a bit more readable but I get the problem of trying to get current.next when current is null so I get a nullpointerexception. Any ideas?

How I would like it would ideally like it written but doesn't work-

listNode current = nameList;
for (String person : names) {
    current.next = new listNode(person);
    current = current.next;
} 

Any help would be appreciated! :)

Was it helpful?

Solution

From my comment:

If you want to use an enhanced for loop i.e. a foreach then you should do the null check inside it.

nameList = null;
listNode current = null;
for (String person : names) {
    if (nameList= null) {
        nameList = new listNode(names.get(0));
        current = nameList;
    } else {
        current.next = new listNode(person);
        current = current.next;
    }
}

Another recommendation: use CamelCase for your class names for better code readability, listNode class should be ListNode and listManager class should be ListManager.


By @corsiKa's comment, this won't be a best suited case to use an enhanced for loop (in fact, this really depends on how many items we're talking about to measure the performance impact, but that's outside of this question). To make it a more generic algorithm that can even supports List (the generic interface implemented by ArrayList), you can use an Iterator instead:

Iterator<String> iterNames = names.iterator();
if (iterNames.hasNext()) {
    nameList = new listNode(iterNames.next());
    listNode current = nameList;
    while (iter.hasNext()) {
        current.next = new listNode(iterNames.next());
        current = current.next;
    }
}

Note that if you still want to stick with the enhanced for loop approach, there's no way to remove the null check from it.

OTHER TIPS

  for (int i = names.size() - 1; i >= 0; i--){
     person = new ListNode(names.get(i), person);
  }

going backward makes it much easier

Problem 1

Without having your full code to compile (which might be more than necessary anyway) it's difficult to debug. One thing to note is your two loops are not identical.

The first does this:

for (int i = 1;  i < names.size(); i++) {

The second does this

for (String person : names) {

Which is essentially the same as

for (int i = 0;  i < names.size(); i++) {
    String person = names.get(i);

Note that your loop starts at 1 but the generated loops starts at 0.

Problem 2

That aside, your problem is most likely that your second part of code doesn't seem to create a new node for your header node (namesList).

The solution

This is not an appropriate place to use an enhanced for loop. Enhanced for loops should only be used when identical operations will be applied to every element in your structure. In this case, you treat the first one as a special case, which complicates code. Your first loop, the one starting at 1, is the ideal way to solve this problem.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top