2
\$\begingroup\$

I have managed to implement an ADTStack with a circular doubly linked list. I would like to know whether there are things I could have done better or, I should improve.

public class QueueCircularDLL<T> implements ADTQueue<T> {    
    private ListElement<T> head;
    private ListElement<T> tail;

    public QueueCircularDLL(){
        head=null;
        tail=null;
    }

    public void enq (T element){
        ListElement<T>newElement=new ListElement<T>(element);
        if (empty()){
            head= newElement;
            tail= head;
        }

        tail.setNextElement(newElement);
        newElement.setPrevElement(tail);
        tail=tail.getNextElement();
        tail.setNextElement(head);
        head.setPrevElement(tail);

    }

    public void deq(){
        if (empty()){
            throw new RuntimeException("queue is empty");
        }

        if (head==tail){
            //head=null;
            head=null;
            tail=null;

        }
        else{
        head=head.getNextElement();
        head.setPrevElement(tail);
        tail.setNextElement(head);
        }

    }

    public T front(){
        if(empty()){
            return null;
        }
        return head.getElement();
    }

    public boolean empty(){
        return (head==null);
    }

    private static class ListElement<T>{
        private T element = null;
        private ListElement<T> nextElement = null;
        private ListElement<T> prevElement = null;

        public ListElement(T element) {
            this.element = element;
        }

        public T getElement() {
            return element;
        }

        public ListElement<T> getNextElement() {
            return nextElement;
        }

        public void setNextElement(ListElement<T> element) {
            this.nextElement = element;
        }
        //für zirkuläre doppelt verkettete liste
        public ListElement<T>getPrevElement(){
            return prevElement;
        }
        // für zirkuläre doppelt verkette liste
        public void setPrevElement(ListElement<T> element){
            this.prevElement=element;
        }



    }

    public static void main(String[] args) {
        QueueCircularDLL<Integer> queue = new QueueCircularDLL<Integer>();

        for (int i = 0; i < 10; ++i) {
            queue.enq(i);
        }

        while (!queue.empty()) {
            System.out.println(queue.front());
            queue.deq();
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

The class doesn't expose a method that let's users benefit from the circular property. The enq, deq, front, empty methods could work just fine without the circular property. As such, it's an unnecessary complexity. If you want to use a circular linked list, then expose some methods that need or benefit from the circular property.

The class QueueCircularDLL<T> implements the ADTQueue<T> interface, but it's not clear which methods belong to the interface. It's strongly recommended to make that clear using the @Override annotation.

In front, instead of returning null when the list is empty, a more common practice is to throw java.util.NoSuchElementException, as done by the various linked list and queue implementations in the JDK.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ hey, first of all, thx for your feedback. \$\endgroup\$ – user3683131 Jul 18 '15 at 11:32
1
\$\begingroup\$

Constructor needed?

First of, these are not needed because Java null initializes object references.

public QueueCircularDLL(){
    head=null;
    tail=null;
}

Meaning, you do not need a constructor in this code.

Reference

Note: It can be considered bad practice, if you prefer to explicity state they are null then do it in the same line as initialization.


Code Conventions

Normally if-else would look like:

if (statement) {
    // if true
} else {
    // if false
}

Reference


Javadoc

Almost every programmer out there will tell you to write it before or during development, which can be hard and time-consuming when still learning. However, it is good practice to write it once you are "done" with the code. Gives you some time to reflect upon what you have done.

Further Read

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$
  • You mix spaces-around-operators styles. Java best practices is to use spaces.
  • It's also best practices to prefix names of methods that return a boolean with verbs like is, has, contains, ... hence:

    public boolean isEmpty()
    

    Just empty() could also mean that something is cleared with the returned boolean as indicator for success.

  • I do, but I'm not sure whether many others understand the German comments here. :-)
| 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.