Inserting nodes into a singly linked list based on size of number

Question

Given a set of numbers insert them so that they are ordered ie.

add(5)
add(3)
add(6)
add(2)
Output: 6 5 3 2
GetNumber() returns the number of that term
insertAfter() inserts that node after the pointer
insertAtHead() inserts as the head node
Term is the Node which holds the number

How can I refactor this working code?

 boolean inserted = false; 
 Node pointer = head;
 if (pointer == null || term.getNumber() > pointer.data.getNumber()) {
 // If head is empty insert there or if term is larger than head.
 insertAtHead(term);
 } else if(pointer.next == null) {
 insertAfter(term, head);
 } else {
 while (pointer.next != null) {
 //Insert at end.
 if (term.getNumber() > pointer.next.data.getNumber()) {
 insertAfter(term, pointer);
 inserted = true;
 break;
 }
 pointer = pointer.next;
 }
 if (!inserted) {
 //If not inserted it must be the smallest so add to the end.
 insertAfter(term, pointer);
 }
 }
 printList();

Answer 1

• You don't need the else if(pointer.next == null) case, it's covered by if (!inserted).
• Declare variables in as small a scope as possible to increase readability. inserted and pointer aren't needed until the while loop.
• You can save a nesting level if you put your code into a insertSorted method, and then call printList afterwards in the calling method. That way, you can write the initial head insert as a guard clause and thus save the else.
• You could also save the break by adding && !inserted to the while clause.
• many of your comments just repeat the code they comment on, and are thus not really needed.
• // Insert at end. is quite misleading, as the insert doesn't happen at the end of the list.

With all these changes, your code already looks a bit cleaner:

if (head == null || term.getNumber() > head.data.getNumber()) {
 insertAtHead(term);
 return;
}
Node pointer = head;
boolean inserted = false;
while (pointer.next != null && !inserted) {
 if (term.getNumber() > pointer.next.data.getNumber()) {
 insertAfter(term, pointer);
 inserted = true;
 }
 pointer = pointer.next;
}
if (!inserted) {
 //If not inserted it must be the smallest so add to the end.
 insertAfter(term, pointer);
}

You could also return early after inserting, and thus get rid of the inserted variable:

// guard clause for head insert
Node pointer = head;
while (pointer.next != null) {
 if (term.getNumber() > pointer.next.data.getNumber()) {
 insertAfter(term, pointer);
 return;
 }
 pointer = pointer.next;
}
//If not inserted it must be the smallest so add to the end.
insertAfter(term, pointer);

But you still have a separate case for the last insert which is a bit ugly. You should be able to get rid of it with something like this:

// guard clause for head insert
Node pointer = head;
while (pointer.next != null && term.getNumber() < pointer.next.data.getNumber()) {
 pointer = pointer.next;
}
insertAfter(term, pointer);