1

I have an array of characters and I'm trying to convert each character into a node that links to the next node in line. The problem is I keep getting caught in infinite loops and I have no idea why. Here's my code:

String map = "ABBACBCCA";
char[] charArray = map.toCharArray();
ListNode head;
ListNode temp;
ListNode next;

for (int i = 0; i < charArray.length - 1; i++) {
     temp = new ListNode(charArray[i]);
     next = new ListNode(charArray[i+1]);
     temp.next = next;

     if (i == 0) {
          head = temp;
     }
}

And the ListNode class looks like:

class ListNode<T> {
     public T data = null;
     public ListNode next = null;

     public ListNode(T data) {
          this.data = data;
     }
}

It looks like it gets to the last iteration of the for loop and then gets caught in an infinite loop.. Anyone know why?

1
  • There is no proof of the infinite loop. The only provided loop (for loop over the charArray does seem to end well. Other than that code is obviously wrong. Commented Apr 10, 2017 at 4:14

4 Answers 4

1

For starts I would think you would want:

next = new ListNode(charArray[i]);

to be

next = new ListNode(charArray[i+1]);

Something else I noticed:

for (int i = 0; i < charArray.length - 1; i++) {
     temp = new ListNode(charArray[i]);
     next = new ListNode(charArray[i+1]);
     temp.next = next;

          if (i == 0) {
            head = temp;
           }
     }

I don't think this is going to yield what you want. It will not give you A->B->B->A etc etc. more over it would give -> A->B, B->B etc etc. Not sure if that's what you are after.

More over I think this should get ya good:

String map = "ABBACBCCA";
        ListNode<Character> head = null;
        ListNode<Character> newHead = null;
        ListNode<Character> next = null;

        char[] charArray = map.toCharArray();

        head = newHead = new ListNode<Character>(charArray[0]);
        for (int i = 1; i < charArray.length - 1; i++) {
            next = new ListNode<Character>(charArray[i]);

            newHead.next = next;

            newHead = next;
        }

Basically create and link and create and link. (tested out fine for me) Incoming ugly!

System.out.println(head.data);
        ListNode<Character> nextptr = head.next;
        while (true) {

            if (nextptr.next == null) {
                break;
            }
            System.out.println(nextptr.data);
            nextptr = nextptr.next;
        }
Sign up to request clarification or add additional context in comments.

5 Comments

LOL I know that feeling!!
Like Sot was saying. Trying running it with the Debugger in Eclipse. It should work.
I was using the debugger and everything looks fine until it get's to the last iteration then it just goes into an infinite loop
got to be something else getting you. I typed it in, ran in. No problem. I would recommend just typing this all in again. Sometimes that's the best way to gain the AH HA moment.
Try that. That should give you a links to B and B links to B and so on and so forth. Check Mark if it works =)
0

Using the debugger is your best bet, if you want to continue developing your own code. You should probably create some public methods to set the next element of LinkList nodes like I have done in this example. Explaining would be lengthy, so here's the code.

public class Test {

 public static void main(String[] args) {
    ListNode<Character> head = new Test().arrayToLinkList();

    while ((head = head.nextNode()) != null) {
        System.out.println(head.readData());
    }
 }

 public ListNode<Character> arrayToLinkList() {
    String map = "ABBACBCCA";
    char[] charArray = map.toCharArray();
    ListNode<Character> head, next;
    head = new ListNode<Character>(charArray[0]);
    next = head;
    for (int i = 0; i < charArray.length - 1; i++) {
        next = next.next(new ListNode<Character>(charArray[i + 1]));
    }
    return head;
 }
}

class ListNode<T> {
 private T data = null;
 private ListNode<T> next = null;

 public ListNode(T data) {
    this.data = data;
 }

 public ListNode<T> next(ListNode<T> next) {
    this.next = next;
    return this.next;
 }

 public ListNode<T> nextNode() {
    return this.next;
 }

 public T readData() {
    return data;
 }
}

Comments

0

Your reference variables temp & next are assigned to new Objects during each iteration and you loose track of your next pointers. You could have figured out this problem using debugger as others suggested. Here is the working example.

public class Test {

    public static void main(String args[]) {
        String map = "ABBACBCCA";
        char[] charArray = map.toCharArray();
        ListNode<Character> head = null, temp = null;
        for (int i = 0; i < charArray.length; i++) {
            ListNode<Character> obj = new ListNode<Character>(charArray[i]);
            if (temp != null) {
                temp.next = obj;
            } else {
                head = obj;
            }
            temp = obj;
        }
        // Print the list
        while (head != null) {
             System.out.println(head.data);
             head = head.next;
        }
    }
}

class ListNode<T> {
    public T data = null;
    public ListNode<T> next;
    public ListNode(T data) {
        this.data = data;
        this.next = null;
    }
}

Comments

0

This is probably one of the simpler ways to build your linked list:

String map = "ABBACBCCA";

ListNode<Character> head = null;
ListNode<Character> tail = null;

for (char c:map.toCharArray()) {
  final ListNode<Character> node = new ListNode<>(c);
  if (head == null) {
    head = node;
  } else {
    tail.next = node;
  }
  tail = node;
}

While taking care of your type parameters everywhere

class ListNode<T> {
  public T data = null;
  ListNode<T> next = null;

  public ListNode(T data) {
    this.data = data;
  }
}

Not to mention that you can fully leverage Java api's as well:

LinkedList<Character> ll =
  "ABBACBCCA".chars()
    .mapToObj(i->(char)i) // takes care of boxing char
    .collect(Collectors.toCollection(LinkedList<Character>::new));

Also to loop through your ListNode's, you could consider adding:

class ListNodeIterator<T> implements Iterator<T> {
  private ListNode<T> current;

  public ListNodeIterator<T> ListNodeIterator(ListNode<T> node) {
    current = node;
  }

  public boolean hasNext() {
    return current.next != null;
  }

  public T next() {
    current = current.next;
    return current.data;
  }
}

With following modification:

class ListNode<T> implements Iterable<T> {
  public T data = null;
  public ListNode<T> next = null;

  public ListNode(T data) {
    this.data = data;
  }

  public Iterator<T> iterator() {
    return new ListNodeIterator<>(this);
  }
}

So you can use it as follows:

for (char c:head) {
  System.out.println("Character: "+c);
}

or even

head.forEach(c->{System.out.println("Character: "+c);});

Well ... wasn't that a nice trip through Javaland?

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.