Skip to main content
Improve the explanation
Source Link
Anatolii
  • 992
  • 5
  • 17
// if the new valueelement is lessnot greater than the next one 
// then insert it between the current element and the next one
if (data <= current.next.data) {
    DoublyLinkedListNode temp = current.next;
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    temp.prev = nodeToInsert;
    nodeToInsert.next = temp;
    break;
}
// if the new value is less than the next one then insert it between the current element and the next one
if (data <= current.next.data) {
    DoublyLinkedListNode temp = current.next;
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    temp.prev = nodeToInsert;
    nodeToInsert.next = temp;
    break;
}
// if the new element is not greater than the next one 
// then insert it between the current element and the next one
if (data <= current.next.data) {
    DoublyLinkedListNode temp = current.next;
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    temp.prev = nodeToInsert;
    nodeToInsert.next = temp;
    break;
}
Source Link
Anatolii
  • 992
  • 5
  • 17

To me, the main problem with your solution is readability. It took me some time to understand what exactly you're doing to solve a Hackerrank task.

Simplify Code

For instance, consider the following block of yours:

...
if (data < current.data && current.prev == null) {
     current.prev = nodeToInsert;
     nodeToInsert.next = current;
     return nodeToInsert;
}
...

Doing it in a while loop is a bit confusing as you can directly compare data with head.data - as per problem statement the list is sorted and so the smallest element is in the current head. Move it out of the loop. Additionally, remove current.prev == null as the head is the first element by definition.

if (data < head.data) {
    head.prev = nodeToInsert;
    nodeToInsert.next = head;
    return nodeToInsert;
}

Another block that can be simplified is:

if (data >= current.data && current.next == null) {
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    break;
}

Here, you check if current is a tail (last element) and then add your nodeToInsert to it. But then data >= current.data is redundant since current.next == null is sufficient to say whether an element is a tail or not. Hence:

if (current.next == null) {
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    break;
}

Or even here:

if (data >= current.data && data <= current.next.data) {
    DoublyLinkedListNode temp = current.next;
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    temp.prev = nodeToInsert;
    nodeToInsert.next = temp;
    break;
}

You can remove data >= current.data as it's given (initially current = head and so the check was made before the loop started):

if (data <= current.next.data) {
    DoublyLinkedListNode temp = current.next;
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    temp.prev = nodeToInsert;
    nodeToInsert.next = temp;
    break;
}

Add useful comments

You could've commented crucial blocks of your code better to make it easier to understand what you're doing. For instance:

// if current is the last element then add the new element after it
if (current.next == null) {
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    break;
}

Or

// if the new value is less than the next one then insert it between the current element and the next one
if (data <= current.next.data) {
    DoublyLinkedListNode temp = current.next;
    current.next = nodeToInsert;
    nodeToInsert.prev = current;
    temp.prev = nodeToInsert;
    nodeToInsert.next = temp;
    break;
}

Static method

Well, I know, Hackerrank forces you to use the static sortedInsert() method so there's nothing you can do about it.