Would this be appropriate design for this situation?

Question

I am implementing the remove(Object o) method in a unsorted linked list and a sorted linked list. Both these classes extend AbstractLinkedList(code reuse)

Here's my code for remove(Object o) in unsorted linked list

@Override
public void remove(E value) {
    if(front != null) {
        if(front.data.equals(value)) {
            front = front.next;
        } else {
            ListNode<E> current = front;
            boolean hasRemovedElement = false;
            while(current.next != null && !hasRemovedElement) {
                if(current.next.data.equals(value)) {
                    current.next = current.next.next;
                    hasRemovedElement = true;
                }
                current = current.next;
            }
        }
    }
}

And my code for remove(Object o) in sorted linked list

public void remove(E value) {
    if(front != null) {
        ListNode<E> current = front;
        if(front.data.equals(value)) {
            front = front.next;
        } else {
            boolean hasRemovedElement = false;
            while(current.next != null && !hasRemovedElement 
                    && current.next.data.compareTo(value) >= 0) {
                if(current.next.data.equals(value)) {
                    current.next = current.next.next;
                    hasRemovedElement = true;
                }
                current = current.next;
            }
        }
    }

What I noticed right away was the code in both of my methods are pretty much identical. The remove(Object o) in sorted linked list just has one more conditional check saying if the next element is less than the value you're trying to remove, there is no way that element you are trying to remove is in the list. What I did to take advantage of code reuse was implementing a overloaded version in AbstractLinkedList that both lists could call, that is

@Override
protected void remove(E value, E toCheckAgainst) {
    if(front != null) {
        ListNode<E> current = front;
        if(front.data.equals(value)) {
            front = front.next;
        } else {
            boolean hasRemovedElement = false;
            while(current.next != null && !hasRemovedElement 
                    && current.next.data.compareTo(toCheckAgainst) >= 0) {
                if(current.next.data.equals(value)) {
                    current.next = current.next.next;
                    hasRemovedElement = true;
                }
                current = current.next;
            }
        }
    }

And inside the unsorted linkedlist

@Override
public void remove(E value) {
      remove(value, CHECKER)
}

But my issue is what should i make this CHECKER in unsorted linked list? In terms of the sorted linked list, this checker could just be the value again. The checker's role is basically stop iterating if you have reached a value less than the checker(sorted). What value can i make the checker to not have it taken any effect on unsorted linked list? I tried using an int like MIN POSSIBLE = -9999999 but you can't compare ints to generics. Or is it better in terms of design just to have the pretty much the same implementation in both classes.

Answer 1

What I recommend is to leave the methods as they are. Code re-use should be weighed against readability etc.

But suppose you want to do this anyway, I'd use a sort of Tester (or in Java 8, a Predicate). You declare an interface like this in your superclass:

protected interface Tester<E> {
    public boolean test(E testObj);
}

Then your general removal method would be:

protected void remove(E value, Tester<E> extraCondition) {
    if(front != null) {
        ListNode<E> current = front;
        if(front.data.equals(value)) {
            front = front.next;
        } else {
            boolean hasRemovedElement = false;
            while(current.next != null && !hasRemovedElement 
                    && extraCondition.test(current.next.data) {
                if(current.next.data.equals(value)) {
                    current.next = current.next.next;
                    hasRemovedElement = true;
                }
                current = current.next;
            }
        }
    }
}

Then in your unsorted list you'd have:

@Override
public void remove(final E value) {
    remove(value, new Tester<E>() {
        public boolean test(E testObj) {
            return true;
        }
    });
}

And in your sorted list:

public void remove(final E value) {
    remove(value, new Tester<E>() {
        public boolean test(E testObj) {
            return testObj.compareTo(value) >= 0;
        }
    });
}

So, in the unsorted list you passed a condition that always returns true, and in the sorted list, you passed a condition that returns true based on comparing to the value.

Your idea of testing for null and only then comparing would work, but:

• It makes it even more unreadable.
• It only works for this particular case. What if you have a list that's sorted in descending order? Or has something exotic like a special "stop" element, where you are only allowed to remove elements that come before the "stop" element?

In general, don't put specialized behavior that pertains only to the subclasses you can currently think of, in your superclass, just to save some coding in subclasses.