2
\$\begingroup\$

The list is pretty simple, here is my code:
ILinkedList.java

package datastructures;

public interface ILinkedList<T> {
    public enum posForOperation { BEGGINIG,BEFORE ,AFTER ,END }
    public class Node<T>{
        T data;
        Node next;
        public Node(T data){
            this.data = data;
            this.next = null;
        }
    }
    public Node newNode(T data);
    public void add(posForOperation insert_at, T data);
    public void add(posForOperation insert_at, T data, Node node);
    public void delete(posForOperation delete_at);
    public void delete(posForOperation delete_at, Node node);
    public void deleteAtPos(int pos);
    public Node search(T data);
    public void print(); // for debugging
}

LinkedList.java

package datastructures;

public class LinkedList<T> implements ILinkedList<T> {
    private Node head;
    private Node end;
    public LinkedList(T data){
        Node new_node = new Node(data);
        head = new_node;
        end  = new_node;
    }
    @Override
    public Node newNode(T data) {
        return new Node(data);
    }

    @Override
    public void add(ILinkedList.posForOperation insert_at, T data) {
        Node new_node = newNode(data);
        if(insert_at == ILinkedList.posForOperation.BEGGINIG){
            new_node.next = head;
            head = new_node;
        }
        else if(insert_at == ILinkedList.posForOperation.END){
            end.next = new_node;
            end = new_node;
        }
    }

    @Override
    public void add(ILinkedList.posForOperation insert_at, T data, Node node) {
        if(insert_at == ILinkedList.posForOperation.BEFORE){
            Node tmp = head; 
            while(tmp.next != node && tmp.next != null){
                tmp = tmp.next;
            }
            Node new_node = newNode(data);
            new_node.next = node;
            tmp.next = new_node;
        }
        else if(insert_at == ILinkedList.posForOperation.AFTER){
            Node new_node = newNode(data);
            new_node.next = node.next.next;
            node.next = new_node;
        }
        else // in case of misuse
            add(insert_at, data);
    }

    @Override
    public void delete(ILinkedList.posForOperation delete_at) {
        if(delete_at == ILinkedList.posForOperation.BEGGINIG){
            head = head.next;
        }
        if(delete_at == ILinkedList.posForOperation.END){
            Node tmp = head;
            while(tmp.next != end && tmp != end){ // the second condition is in case that head = end
                tmp = tmp.next;
            }
            tmp.next = null;
            end = tmp;
        }
    }

    @Override
    public void delete(ILinkedList.posForOperation delete_at, ILinkedList.Node node) {
        if(node.next == null)
            delete(ILinkedList.posForOperation.END);
        else if(delete_at == ILinkedList.posForOperation.AFTER){
            node.next = node.next.next;
        }
        else if(delete_at == ILinkedList.posForOperation.BEFORE){
            Node tmp = head;
            while(tmp.next != node){
                tmp = tmp.next;
            }
            tmp.next = node.next;
        }
        else //in case of misuse
            delete(delete_at);
    }

    @Override
    public ILinkedList.Node search(T data) {
        Node tmp = head;
        while(tmp.data != data && tmp != null){
            tmp = tmp.next;
        }
        return tmp;
    }

    @Override
    public void deleteAtPos(int pos) {
        Node tmp = head;
        for (int i = 0; i < pos-1 && tmp != null; i++){
            tmp = tmp.next;
        }
        delete(ILinkedList.posForOperation.AFTER, tmp);
    }

    @Override
    public void print() {
        Node tmp = head;
        while(tmp != end){
            System.out.println(tmp.data);
            tmp = tmp.next;
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Not a review, but you spelt beginning incorrectly. \$\endgroup\$ – David Jun 15 '16 at 13:29
  • 1
    \$\begingroup\$ There is no reason for ILinkedList name. You could just call List. \$\endgroup\$ – rdllopes Jun 15 '16 at 20:23
1
\$\begingroup\$

Initial impressions:

  • public Node newNode(T data) shouldn't be on the interface, why would anything outside of the list need to construct one?
  • print shouldn't be on the interface, I know you say it's for debugging, but a better approach would be to provider an iterator so that something outside the list could iterate the items and print them.
  • I don't like the fact that Node is visible outside of the list class. It reveals too much of the underlying functionality. What happens if I call 'newNode' then pass the contents of it to the delete method?
  • One of your delete methods takes in a 'delete_at' parameter, then ignores it in certain conditions and deletes the last member of the list instead...

    public void delete(ILinkedList.posForOperation delete_at, ILinkedList.Node node) {
            if(node.next == null)
                delete(ILinkedList.posForOperation.END);
            else if(delete_at == ILinkedList.posForOperation.AFTER){
    
  • void deleteAtPos(int pos), there's no way for a caller to really know where in the list an item is (unless they count as they insert them?), so what's the purpose of this method?

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.