Search code examples
javadata-structureslinked-listimplementationabstract-data-type

Code Review for Linked List add(i, x) Method


I'm currently trying to brush up on ADT implementations, specifically an implementation for a Linked List (I'm using Java 5 to do this).

I have two questions:

(1) Is this implementation I've written for add(i, x) correct and efficient?

public void add(int i, Object x) {

    // Possible Cases:
    //
    //     1. The list is non-empty, but the requested index is out of
    //        range (it must be from 0 to size(), inclusive)
    //
    //     2. The list itself is empty, which will only work if i = 0
    //
    // This implementation of add(i, x) relies on finding the node just before
    // the requested index i.

    // These will be used to traverse the list
    Node currentNode = head;
    int indexCounter = 0;

    // This will be used to mark the node before the requested index node
    int targetIndex = i - 1;

    // This is the new node to be inserted in the list
    Node newNode = new Node(x);

    if (currentNode != null) {

        while (indexCounter < targetIndex && currentNode.getNext() != null) {

            indexCounter++;
            currentNode = currentNode.getNext();
        }

        if (indexCounter == targetIndex) {

            newNode.setNext(currentNode.getNext());
            currentNode.setNext(newNode);

        } else if (i == 0) {

            newNode.setNext(head);
            head = newNode;
        }

    } else if (i == 0) {

        head = newNode;
    }     
}

(2) I found this method very challenging to implement. To be honest, it took me several days. This is difficult to admit, because I love programming and consider myself at intermediate level in several languages and platforms. I've been programming since I was 13 (Applesoft BASIC on an Apple IIc!) and have a degree in CS. I currently work as a software tester and have plans to become a developer at some point. So the second part of my question is: am I fooling myself that this is the type of work I would excel at, or does pretty much everyone find this kind of problem challenging? Something tells me even seasoned developers, being faced to implement this method, would find it challenging.

Thanks for your feedback and advice on the second part.


Solution

  • I think it's a good start...A few suggestions:

    • I think your currentNode == null cases should be taken care of at the start, and then return. I don't like everything being inside the "if (currentNode != null)"
    • You should keep track of the size of your linked list somewhere, so that you can easily just check if i > list_size
    • I wouldn't bother renaming "i-1" to targetIndex
    • Don't create the newNode until you need to
    • Write unit tests, then you can easily change things around and know that your implementation still works.
    • Don't simply ignore illegal arguments. If index i < 0 or > size, throw an IllegalArgumentException or IndexOutOfBoundsException (thanks @JB Nizet)