0

Can't seem to add a last element properly. I hold the last item in a temp node, then create a new node. Then I link both previous and next for each node, then point the last node to a new null node. But that null node doesn't seem to be part of the list when I go to my print() method.

Seems like it should be as easy as my push method, but I just can't seem to get it work like it.

public class LinkedListDeque {

public DoubleNode first = new DoubleNode(null);
public DoubleNode last = new DoubleNode(null);
public DoubleNode temp;
public int N;

LinkedListDeque() {
    first.next = last;
    last.prev = first;

}

public static void main(String[] args) {

    LinkedListDeque link = new LinkedListDeque();

    link.push("banana");
    link.printList();

    link.enqueue("gorilla");
    link.printList();


    link.enqueue("spam");

}


//nested class

private class DoubleNode {

    String item;
    int counter = 0;
    DoubleNode next;
    DoubleNode prev;

    DoubleNode(String i) {
        this.item = i;
    }

}

public void push(String item) {

    System.out.println("\npush()\n******");
    if (first.item == null) {
        first.item = item;
        first.counter++;
    } else {

        System.out.println("last.item = " + last.item);
        DoubleNode node = new DoubleNode(item);
        first.prev = node;
        node.next = first;
        first = node;

    }

}



 public void enqueue(String item) {
    System.out.println("\nenqueue()\n***********");
    System.out.println("adding \"" + item + "\" to the end");

    if (last.item == null) {
        DoubleNode node = new DoubleNode(null);                     //holds null node to end list
        last.item = item;
        last.next = node;
    } else {
        DoubleNode node = new DoubleNode(null);
        System.out.println("node = " + node.item);                  //= correct item

        temp = last;
        last = new DoubleNode(item);                                //creating a new last node

        System.out.println("temp = " + temp.item);                  //corect
        //reconnect the links
        temp.next.item = last.item;

        System.out.println("temp.prev = " + temp.prev.item);        //correct
        System.out.println("temp.next = " + temp.next.item);        //correct
        System.out.println("last = " + last.item);                  //correct
        System.out.println("last.prev = " + last.prev);             //correct

        last.prev = temp;

        System.out.println("last.prev = " + last.prev.item);        //correct

        last.next = node;
        System.out.println("last.next = " + last.next.item);        //= null to end list

        System.out.println("\n\nfirst = " + first.item);                                //correct
        System.out.println("first.next = " + first.next.item);                          //correct
        System.out.println("first.next.next = " + first.next.next.item);                //correct
        System.out.println("first.next.next.next = " + first.next.next.next.item);      //"null pointer exception"
}

public void printList() {
    System.out.println("\nprintList():\n********");
    temp = first;

    int i = 0;
    if (first.item == null) {
        temp = first.next;
    }
    System.out.println("temp = " + temp.item);
    while (temp.item != null) {
        i++;
        System.out.println(i + " " + temp.item);
        temp = temp.next;
    }
    System.out.println();
}
Travelsbyfire
  • 35
  • 1
  • 10
  • 1
    Why does the `enqueue` method contain print statements? – arcadeblast77 Sep 17 '19 at 18:25
  • 1
    Your `first` node and `last` node are being treated as two separate lists, where `last` is the first node of its own list. You need to link up the `last` and `first` nodes when you add the first node into the list. – Mihir Kekkar Sep 17 '19 at 18:26
  • 1
    To add a new node at the end of a circular doubly linked list. First you need to create a new node. Set previous of new node to last node and next to head. The next step is to set next of last node to new node and set previous of head to new node. – Yoshikage Kira Sep 17 '19 at 18:27
  • @arcadeblast77 to test whether or not the intention of each line of code is doing what I want it to do. – Travelsbyfire Sep 17 '19 at 18:28
  • 1
    @Travelsbyfire Using a debugger is way better. Get comfortable with it. – Yoshikage Kira Sep 17 '19 at 18:29
  • @MihirKekkar I'm keeping a null node at the end because that's how my list will determine where the end should be I guess. – Travelsbyfire Sep 17 '19 at 18:30
  • @Goion It's not a circular doubly linked list – Travelsbyfire Sep 17 '19 at 18:30
  • @Goion probably, but I've never used one, so I'm not sure where I'd even begin – Travelsbyfire Sep 17 '19 at 18:31
  • 1
    When you add your very first node to the list, `first` and `last` should hold the same node because the first and last element would both be the one you have just added. In your `push()`, you are only assigning the first node to `first`, so `last` never actually gets connected to the list. Hence when you print out the whole list starting from `first`, it won't be able to access nodes linked from `last`. – Mihir Kekkar Sep 17 '19 at 18:32
  • 1
    this is a good start [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Yoshikage Kira Sep 17 '19 at 18:32
  • 2
    Suggestion. Take a paper and pen and draw a step by step diagram of what should happen. It would help you a lot. I had an assignment where I had to insert node in doubly circular list sorted. Drawing helped me a lot. – Yoshikage Kira Sep 17 '19 at 18:34
  • @Goion thanks, I'll definitely check that out, get's tiring to keep writing system.out.println(); haha – Travelsbyfire Sep 17 '19 at 18:36
  • @Goion, Yeah I've been drawing it constantly, that's how I came up with this code, but drawing abstract maybe doesn't work well for me, idk – Travelsbyfire Sep 17 '19 at 18:37

2 Answers2

2

Let's see what is actually happening: the initial state:

  • first contains null, first.next is last, last contains null
  • push: first does not contain null any more, still points to last,with null
  • enque: last.item is null, so the first cae is triggered, now the list is like: banana -> gorilla -> null and last points to gorilla
  • enque again: now the else is triggered. If you have a look at the code, you will notice that temp.next is not touched anywhere. This means that the node that was the last node before the enque,and was copied to temp still points to the null node.
  • this finally results in a null pointer exception.

What is missing: something like temp.next=last, after the last node is created.

What actually happens looks like this:

---> last ---> closing-null

---> temp ---> closing-null

---> last

It seems you could implement this more cleanly without the null nodes closing the list.

Then you could do something like this:

node=new Node(item);
last.next=node;
node.prev=last;
last=node;
g_bor
  • 1,077
  • 6
  • 14
  • if they aren't connected, why would first.next.next = last? – Travelsbyfire Sep 17 '19 at 18:33
  • 1
    Yes, in fact the interlinking is missing in some other places, like the last null node prev is not added anywhere in enque. – g_bor Sep 17 '19 at 18:34
  • I'm not sure if this is what you mean, but at the very top, first.next = last – Travelsbyfire Sep 17 '19 at 18:38
  • So the only problem I have with this code is that I still get a null pointer exception. I can't even understand why though – Travelsbyfire Sep 17 '19 at 18:46
  • But I do have the temp.next. item = last.item as the first thing I do to reconnect the links – Travelsbyfire Sep 17 '19 at 19:09
  • 1
    temp.next.item is the string contained in the node pointed by temp.next. That is the item that had the null pointer previously. The temp.next pointer is not updated, so it is still pointing to the item that was the closing null item in the previous version of the queue. You update it's content, but it gets detached from the list, and it points to nowhere. You can not reach last any more from first after the else ran. Does that make sense? – g_bor Sep 17 '19 at 19:23
  • 1
    oh hell yeah, it was just that one single bit of code. I just changed it to temp.next = last and now it works perfectly. Idk why I thought I needed to change the value – Travelsbyfire Sep 17 '19 at 19:28
1

I won't give you the whole code but I will visualize it. You can come up with code easily after this.

Next is ---> and Previous is <--- and last points towards last node and first towards first node

Lets say you have this list.

Banana ---> Orange ---> Gorilla ---> null
       <---        <---         
  ^                       ^
  |                       |
first                    last
// First's previous and last's next is null.

You want to add Mango at the end. You create a new node

DoubleNode node = new DoubleNode("Mango");
<--- Mango --->
// Note: When you create a new node by default both next and previous are null. 
// You don't need to point them to null later

Step 1:

last.next(newNode);
Banana ---> Orange ---> Gorilla ---> Mango 
       <---        <---    
  ^                       ^
  |                       |
first                    last  

Step 2:

newNode.previous(last);
Banana ---> Orange ---> Gorilla ---> Mango 
       <---        <---         <---
  ^                       ^
  |                       |
first                    last

Now we have new last so we will update last

last = newNode
Banana ---> Orange ---> Gorilla ---> Mango 
       <---        <---         <---
  ^                                    ^
  |                                    |
first                                last

From constructor we know Mango's next is already null so newNode.next(null); is unnecessary.

Reasons you could be getting nullpointerexception

After adding first element your list looks like this

null <--- Banana ---> null      null
             ^                   ^
             |                   |
           first                last

Technically you should make both first and last point towards banana since you don't. When you enqueue something it would be let's say gorilla. It would be

null.item = "Gorilla"
null.next = null
Yoshikage Kira
  • 1,070
  • 1
  • 12
  • 20
  • So would (first.next = last) and (last.prev = first;) make sure that both first and last are pointing to banana? – Travelsbyfire Sep 17 '19 at 19:21
  • 1
    @Travelsbyfire It is up to you. If you do that your list would become circular. You could just point last towards banana and it would fix issue. If your last is null assume that your list is completely empty. – Yoshikage Kira Sep 17 '19 at 19:23
  • 1
    @Travelsbyfire Even after pointing last towards banana the algo I gave in answer to enqueue would still work. – Yoshikage Kira Sep 17 '19 at 19:27
  • yeah it totally makes sense. g_bor actually pointed out to me the line temp.next.item = last.item didn't actually change my node, rather the pointer, and after changing that line to temp.next = last, it stopped giving me the null pointer exception. – Travelsbyfire Sep 17 '19 at 19:33
  • @Travelsbyfire Right`temp.next.item = last.item;` temp points towards last element. Its next would return `null`. – Yoshikage Kira Sep 17 '19 at 19:38
  • 1
    Thanks for the illustration, this stuff gets really confusing quickly without something like that, ive decided to include mango in my arbitrary variable values – Travelsbyfire Sep 17 '19 at 20:41
  • @Travelsbyfire Nice one – Yoshikage Kira Sep 17 '19 at 20:45