I have a LinkedList full of Book objects which are stored in Nodes, which contain the year of publication and the title of the book. I am trying to sort the list so that it is arranged from oldest year to most recent year, however, after the first Book, the rest of my books now have the same title and year of publication. I believe it has something to do with when I make the swap using my setBook method.

public void sortByYear(){
    Node current = head;
    Node next = null;

    if(isEmpty()){ //if the head is null
        return;
    }

    while(current != null){
        next = current.getNext();

        while(next != null){
            if(current.getBook().getYear() > next.getBook().getYear()){
                Book temp = current.getBook();
                current.setBook(next.getBook().getYear(), next.getBook().getTitle());
                next.setBook(temp.getYear(), temp.getTitle());
                // current.getBook() = next;
                // next.getBook() = temp;
            }

            next = next.getNext();
        }

        current = current.getNext();
    }

}

CodePudding user response：

A few issues:

• Your setBook method -- taking a year and title -- seems to mutate the book that is assigned to the node. This is not really the right approach. You should define a method that leaves the books untouched, but assigns a different book (all together) to a node.

• Bubble sort will always start the inner loop from the first item in the list. The outer loop would only serve to count the number of times the inner loop has to kick in. This is not how you have implemented it. If the minimum node is not in the first or second place, your algorithm will never move it all the way back to the first position.

I would suggest to take a different approach and only mutate the next references in the list, so that you swap nodes as a whole, without having to touch the data at all. For this to work, you need a prev reference that walks "behind" the current node, so that you can rewire the preceding node to the node that gets swapped with the current node.

Here is an implementation:

    public void sortByYear(){
        if (head == null || head.getNext() == null) {
            return;
        }

        for (Node iter = head; iter != null; iter = iter.getNext()) {
            Node prev = null;
            Node current = head;
            for (Node next = current.getNext(); next != null; next = current.getNext()) {
                if (current.getBook().getYear() > next.getBook().getYear()) {
                    if (prev == null) {
                        head = next;
                    } else {
                        prev.setNext(next);
                    }
                    current.setNext(next.getNext());
                    next.setNext(current);
                    prev = next;
                } else {
                    prev = current;
                    current = next;
                }
            }
        }
    }

Something you could still work on

It would become even nicer if you would make your Node class comparable. That way the if statement can become more generic and less tied to your book-related implementation.