CODEWITHSUNDEEP
javalinked-listdoubly-linked-list
user3076352
user3076352

Reputation:

Java - Manual linked list, removing current node

So I've implemented a linked list from scratch and am trying to remove the current node (cursor). When I run the program and try to remove the current node, I don't receive any errors, but then I'll try to print the current node (which should now be the next or previous) and it prints the one that should've been removed.

Upvotes: 1

Views: 728

Answers (2)

nem035
nem035

Reputation: 35481

First of all, this line makes no sense:

// ...
}else{
    cursor = cursor.getPrev().getNext(); // THIS LINE - all you say here is 'cursor = cursor'
    cursor = cursor.getNext();
}
// ...

You probably wanted to disconnect the previous node from pointing to cursor and make it point to node after cursor:

// get previous node and set its next to node after cursor
cursor.getPrev().setNext(cursor.getNext());

In this part:

if(cursor.getNext() == null){ //it's the tail
    tail = cursor.getPrev();
}

You never disconnect tail.next by saying tail.next = null so your tail.next will point to cursor after you update it.

And then this line:

else{
    cursor = cursor.getNext().getPrev();  // again no effect
    cursor = cursor.getPrev();
}

Should look like:

// get next node and set its prev to node before cursor
cursor.getNext().setPrev(cursor.getPrev());

Overall, your logic seems much more complicate then it should be. Here is one way of simplifying your code but not changing your logic (still use cursor node)

You can just reorder your if statements a little bit to make things clearer. You should check the edge cases (head and tail) first, then the rest:

if (cursor != null){
    if(cursor.getPrev() == null){ //it's the head
            head = cursor.getNext(); 
            head.setPrev(null); // disconnect the head from current node
    } else if (cursor.getNext() == null) { // it's the tail
            tail = cursor.getPrev();
            tail.setNext(null); // disconnect the tail from current node
    } else { // regular node
            Node prev = cursor.getPrev();
            prev.setNext(next);  // connect previous node to next node
            Node next = cursor.getNext();
            next.setPrev(prev); // connect next node to previous node

    }
    // this part isn't necessary because we are skipping the cursor node
    // so nothing in the list references to it anymore 
    // however it is a good safety measure and it helps the GC a bit
    cursor.setPrev(null); // disconnect cursor from previous node
    cursor.setNext(null; // disconnect cursor from next node
}

I left out the updating of cursor because there is an ambigous situation when cursor is on a middle node and you remove it. The problem is how do you decide to update the cursor to prev or to next?

You don't really need the cursor but I've already congested this answer a lot so i will give you this link and this link to check it out for some good ideas.

As far as formatting your long-prints:

If you are using Eclipse, you can use Ctrl-Shift-F on Windows or Cmd-Shift-F on Mac to automatically format your code :)

Upvotes: 3

drew moore
drew moore

Reputation: 32670

I'm suspicious that your call to 

cursor = cursor.getPrev().getNext();

(assuming cursor is the element in your list you want to delete) isn't doing anything, as cursor should already == cursor.getPrev().getNext()

What I suspect you want to do is 

 cursor.getPrev().setNext(cursor.getNext()); // note SET instead of GET
 cursor.getNext().setPrev(cursor.getPrev());

Upvotes: 1

Related Questions