views:

180

answers:

3

Hello, I am trying to implement the linked list data structure using java, it works fine for insertion or removing first elements but fails to remove the last element via using the removeLast() method.

My Linked List Node class: public class LLNode { String value; LLNode next;

    public LLNode(String value){
        this.value = value;
        this.next = null;
    }

}

My Linked List class that holds a head Node:

public class LL {
    LLNode head;

    public LL(){
        this.head = null;   
    }

    public void insertHead(LLNode input){
        input.next = head;
        this.head = input;
    }

    public void removeFirst(){
        this.head = this.head.next;
    }

    public void removeLast(){
        if (this.head == null || this.head.next == null){
            this.head = null;
            return;
        }
        LLNode current = this.head;
        LLNode tmphead = current;
        while(current.next.next != null){
            current = current.next;
        }
        current.next.next = null;
        this.head = tmphead ;
    }

    public void printAll(){
        LLNode current = this.head;

        while(current != null){
            System.out.print(current.value+" ");
            current = current.next;
        }
        System.out.println();
    }


    public static void main( String[] args){

        LL test = new LL(); 
        LL test2 = new LL();
        String[] eben = {"one","two","three","four","five","six"};

        for(int i =0;i<eben.length;i++){
            test.insertHead(new LLNode(eben[i]));
        }
        test.printAll();
        test.removeFirst();
        test.printAll();
        test.removeLast();
        test.printAll();


    }

}

The output is such:

six five four three two one 
five four three two one 
five four three two one 

even tho it had to be like this:

six five four three two one 
five four three two one 
five four three two

What is wrong with my implementation ?

+1  A: 

current.next.next = null;

should be

current.next = null;


and the implementation fails with a NPE when trying to remove the first element of an empty List

DaveC
+1  A: 

while(current.next.next != null) advances until it's true. At which point you set current.next.next = null;. Which it already is.

z5h
+1  A: 

If you need a lot of code and/or a lot of variables for a task like this, you've probably overcomplicated yourself into a corner. Here's how I would do removeLast():

public void removeLast() {
  // avoid special case: List is empty
  if (this.head != null) {
    if (this.head.next == null) {
      // handle special case: List has only 1 node
      this.head = null;
    } else {
      LLNode prelast = this.head; // points at node before last (eventually)
      while (prelast.next.next != null) prelast = prelast.next;
      prelast.next = null;
    }
  }
}

Untested! Use at your own risk. No refunds of purchase price.

Carl Smotricz