The type <E extends Comparable<E>>
is fine, but there are several other problems:
c
is a Collection
, but you can't sort a Collection
unless that Collection
is a List
, because only List
s allow the arranging and rearranging of elements in specific orders. Other types of Collection
, such as a Set
or bag, do not. You could typecast c
to List
, but still it would be the wrong object to sort. It looks like you want to put the contents of c
into your linked list and then sort that:
SortedLinkedList(Collection<? extends E> c)
{
list.addAll(c);
Collections.sort(list);
}
The list is declared LinkedList<Object>
but should be LinkedList<E>
, (or maybe List<E>
), so that it is declared to contain sortable objects.
The assignment of new LinkedList();
should be new LinkedList<E>()
, which can be shortened to new LinkedList<>()
.
That's enough changes to make the code compile, but let's delve deeper. I infer that what you're trying to do here is create a generic container collection that is a linked list with an invariant that its elements are always maintained in sorted order. In that case, some changes you'll want to make are:
The list
variable should be private
to prevent other classes futzing about with it. If you do not want to re-assign the variable after initialization it would also be nice to make it final
, which protects against accidental re-assignment and clarifies that that is how you're using it.
The constructors should be public
to allow access from other packages.
I'm not sure what you intend the compareTo
method there for. (How do you define a comparison of one entire collection against another?) Possibly it should be removed.
Currently you're both encapsulating and extending LinkedList
, which doesn't make sense. Each instance of your class has two LinkedList
s, one in the list
variable, and one inherited from the parent. You need to decide on one or the other.
If you want to extend LinkedList
then you can get rid of the list
variable entirely and call the superclass methods instead. E.g.:
public SortedLinkedList(Collection<? extends E> c)
{
super.addAll(c);
Collections.sort(this);
}
In this case you will need to override any mutative methods of the parent class to make sure that none of them can be used to subvert the invariant that your class maintains its elements in sorted order. E.g., override add(E element)
and make it insert the new element in the correct place. Whereas add(int position, E element)
should be overridden to throw an UnsupportedOperationException
since inserting an element at a specified index position doesn't make sense, because an element's position in a sorted list is already implied by its value.
A disadvantage of extending LinkedList
is that is possible for new mutative methods to be added to the LinkedList
class in future, which could then allow users to subvert your collection's invariant.
If you want to encapsulate a LinkedList
with your list
variable, then you should delete extends LinkedList<E>
and instead have implements List<E>
.
In this case you will need to provide an implementation for all the methods of the interface, but you can instantly implement most of them correctly by extending one of the abstract skeletal classes that the Java Collections Framework provides, such as AbstractSequentialList
.
Third possibility: neither extend nor encapsulate LinkedList
but write a linked list from scratch.
The line import java.lang.*;
is unnecessary. Everything in the java.lang
package is imported by default.
The following is an example based on the above fixes:
import java.util.*;
public class SortedLinkedList<E extends Comparable<E>>
extends AbstractSequentialList<E> implements List<E> {
private final LinkedList<E> list = new LinkedList<>();
public SortedLinkedList() {}
public SortedLinkedList(Collection<? extends E> c)
{
list.addAll(c);
Collections.sort(list);
}
@Override
public boolean add(E element) {
list.add(element);
Collections.sort(list);
return true;
}
@Override
public ListIterator<E> listIterator(int index) {
// Rather than returning list.listIterator(index) directly, we
// encapsulate it to block the add and set methods:
return new ListIterator<E>() {
private final ListIterator<E> base = list.listIterator(index);
@Override
public boolean hasNext() {
return base.hasNext();
}
@Override
public E next() {
return base.next();
}
@Override
public boolean hasPrevious() {
return base.hasPrevious();
}
@Override
public E previous() {
return base.previous();
}
@Override
public int nextIndex() {
return base.nextIndex();
}
@Override
public int previousIndex() {
return base.previousIndex();
}
@Override
public void remove() {
base.remove();
}
@Override
public void set(E e) {
// prevent unsorting the list
throw new UnsupportedOperationException();
}
@Override
public void add(E e) {
// prevent unsorting the list
throw new UnsupportedOperationException();
}
};
}
@Override
public int size() {
return list.size();
}
}
The bulk of the List
methods get implemented with no effort thanks the magical superclass AbstractSequentialList
and its superclasses. However if you check the source you'll find things you can improve if you override those methods because the inherited implementations are designed principally to minimize effort in extending the class. E.g. to clear the list it iterates each element and carefully removes them one at a time (via ListIterator.remove()
), whereas deferring to the LinkedList
's clear()
method would be faster.
Also, instead of re-sorting the entire list after adding an element, it would be much more efficient to insert it directly in the correct place. You can do this via ListIterator.add
, but I'll leave that to you :)
Another nice feature would be to allow your class to be constructed with a custom Comparator
to be used for sorting elements. This would allow use of element types that do not implement Comparable
, as well as the ability to override the default ordering (e.g., users of the class could supply a Comparator
for case-insensitive ordering of String
s). TreeMap
is an example of a class that supports this sort of feature.
I hope the example is helpful for showing the concepts, anyway.