HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Java Singly Linked List Implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationjavasinglylistlinked

Problem

I've implemented a singly linked list in Java as a practice exercise and would appreciate a review.

LinkedList.java

public class LinkedList {

    private Node head;
    private int size;

    public LinkedList() {
    }

    public void addToEnd(char data) {
        addAtIndex(data, size);
    }

    public void addToBeginning(char data) {
        addAtIndex(data, 0);
    }

    public void addAtIndex(char data, int index) {
        Node temp = new Node(data);
        if (index  size) {
            throw new IndexOutOfBoundsException();
        } else if (index == 0) {
            temp.setNextNode(head);
            head = temp;
            incrementCount();
        } else {
            Node current = head;
            for (int i = 1; i = size) {
            throw new IndexOutOfBoundsException();
        } else if (index == 0) {
            head = head.getNextNode();
            decrementCount();
        } else {
            Node current = head;
            for (int i = 1; i = size) {
            throw new IndexOutOfBoundsException();
        } else {
            Node current = head;
            for (int i = 0; i < index && current != null; i++) {
                current = current.getNextNode();
            }
            return current.getData();
        }
    }

    public void decrementCount() {
        size--;
    }

    public void incrementCount() {
        size++;
    }

    public int getSize() {
        return size;
    }

    public Node getHead() {
        return head;
    }
}


Node.java

public class Node {

    private char data;
    private Node next;

    public Node(char data) {
        this.data = data;
    }

    public char getData() {
        return data;
    }

    public Node getNextNode() {
        return next;
    }

    public void setData(char data) {
        this.data = data;
    }

    public void setNextNode(Node next) {
        this.next = next;
    }
}

Solution

Your code seems good to me. It's readable, well structured and formatted. So just a couple of minor points:

When implementing a list, it's always a good idea to implement the List interface, as it's well thought out.

The main differences to your interface I am seeing:

  • naming: your method names contain an xIndex at the end. This isn't really needed, as the argument name index already suggests this.



  • extra methods: you have a lot of methods which deal with the corner cases of other methods. So eg you have deleteFromBeginning, deleteFromEnd, and deleteFromIndex (same for add and get). This doesn't really seem necessary. List for example only has this for add, as adding happens quite often. It doesn't happen so often to want to delete from the beginning/end though.



  • extra public methods: you also have some methods which are just not needed. decrementCount isn't needed and will cause problems when used, as the size will be wrong then. getHead is also not needed, it just exposes Node, which isn't really needed outside of the list context. Just make these private.



  • you are missing some useful methods and your list is not generic, I'm assuming that both are on purpose as this is just for practice.



Misc

  • Node should be a private class of LinkedList, as it's not needed outside of the list at all.



  • I would use this for fields. It's a question of preference, but I prefer it as it makes it explicit where the variable comes from.



  • initialize size explicitly to 0 in the constructor.

Context

StackExchange Code Review Q#122122, answer score: 5

Revisions (0)

No revisions yet.