patternjavaMinor
Simple Linked List with my own super classes and interfaces
Viewed 0 times
simpleinterfaceswithsuperownclassesandlistlinked
Problem
I have provided a simple implementation of a linked list using my own super classes and interface. The implementation is for adding, deleting obtaining the size of thew list. I have not yet provided the iterator in the current code as it's not in the scope for now.
SimpleList.java
SimpleAbstractLinkedList.java
```
package com.learn.linkedlist;
public abstract class SimpleAbstractLinkedList implements SimpleList{
protected SimpleNode rootElement;
protected int size;
SimpleList.java
package com.learn.linkedlist;
import com.learn.linkedlist.exceptions.IllegalPositionArgException;
public interface SimpleList {
/**add the node at the end of the list
*
* @param node
* @return true if successfully added and list has changed due to addition
*/
public abstract boolean add(E node);
/**add the first occurrence of the node
*
* @param node
* @return true if deletion is successful, in case the node is not present then return false
*/
public abstract boolean delete(E node);
/**add a node at the specified position
*
* @param position
* @param node
* @return
* @throws IllegalPositionArgException in case the position size of the list
*/
public abstract boolean add(int position,E node)throws IllegalPositionArgException;
/**delete the node at the specified position
*
* @param position
* @return
* @throws IllegalPositionArgException in case the position = size of the list
*/
public abstract boolean deleteAt(int position) throws IllegalPositionArgException;
/**get the node the specified position
*
* @param position
* @return node at the position
* @throws IllegalPositionArgException in case the position = size of the list
*/
public abstract E get(int position) throws IllegalPositionArgException;
/**
*
* @return the number of nodes in the list
*/
public abstract int size();
}SimpleAbstractLinkedList.java
```
package com.learn.linkedlist;
public abstract class SimpleAbstractLinkedList implements SimpleList{
protected SimpleNode rootElement;
protected int size;
Solution
SimpleListFirst, let's look at the JavaDoc. It's good that you are including this information. However, the docs are not always in sync with the actual method signature, or that method's supposed meaning. The
delete(E node) method is documented to “add the first occurrence of the node”. Say what? Similarly, there are boolean return values, but the @return part of the JavaDoc does not always explain what this return value means.Only the methods in that interface, but not the interface itself have JavaDocs. Consider explaining what that interface is supposed to signify. That would also be a good place to document whether implementations of
SimpleList are supposed to handle null values.Now, let's have a look at all the methods in that interface:
public abstract boolean add(E node);
public abstract boolean delete(E node);
public abstract boolean add(int position,E node)throws IllegalPositionArgException;
public abstract boolean deleteAt(int position) throws IllegalPositionArgException;
public abstract E get(int position) throws IllegalPositionArgException;
public abstract int size();I don't know why
add returns a boolean value. There is no way that adding an element to a list will not result in the list being changed, so this should always return true. As such, the return value contains no information whatsoever, and should be removed. The java.util.Collection interface does contain a boolean add(E) operation because adding an element to a Set might not change the Set if that Set already contains an equal element. But unless you want to update your SimpleList interface to actually implement java.util.Collection, there is no need for such cargo-culting.It is slightly confusing that you are using an
add overload to insert an element at a given position, but that you did not use an overload for a delete at a certain position. I suggest you change deleteAt(int position) to delete(int position), and possibly rename delete to remove while we're at it. However you rename deleteAt, you should change it to return the removed element – a boolean return value is unnecessary, because you will always perform a deletion as long as the index is inside that list's bounds. However, returning the removed element is sometimes useful, and does not impose additional cost – otherwise, a client wishing to access the deleted element would first have to get it, which will traverse the list twice.Considering that you are implementing a (doubly) linked List abstract data type, I am missing a few operations in this interface:
-
Accessing the first and last element. Adding elements at the beginning and the end. Removing the first and last element. Suggested methods:
public E getFirst()
public E getLast()
public E removeFirst()
public E removeLast()
public void addFirst(E value)
public void addLast(E value)Note that the
getX and removeX methods should throw an error if size() == 0.-
Replacing the element at a certain position. Suggested method:
public E set(int pos, E newValue)-
Using the list as an
java.lang.Iterable.-
Joining lists, and adding mutiple values. This could be done with a method like
public void addAll(Iterable values)
public void addAll(int pos, Iterable values)-
A simple
boolean isEmpty() accessor can also be helpful in many circumstances.-
A
boolean contains(E value) and int index(E value) method to search the list. Currently, this would have to be performed manually, although delete(E value) can be used as a destructive contains.An interface should try to strike a good balance between usefulness and minimalism. While all the methods I just suggested can be implemented in terms of the methods your interface already contained, doing so would be unnecessarily inefficient (e.g.
E set(int pos, E newValue) could be implemented as E oldValue = get(pos); deleteAt(pos); add(pos, newValue); return oldValue, which would traverse the list three times, whereas a built-in version can do with only one traversal).Code Snippets
public abstract boolean add(E node);
public abstract boolean delete(E node);
public abstract boolean add(int position,E node)throws IllegalPositionArgException;
public abstract boolean deleteAt(int position) throws IllegalPositionArgException;
public abstract E get(int position) throws IllegalPositionArgException;
public abstract int size();public E getFirst()
public E getLast()
public E removeFirst()
public E removeLast()
public void addFirst(E value)
public void addLast(E value)public E set(int pos, E newValue)public void addAll(Iterable<E> values)
public void addAll(int pos, Iterable<E> values)Context
StackExchange Code Review Q#61603, answer score: 5
Revisions (0)
No revisions yet.