3
\$\begingroup\$

I implemented this solution for the problem. Please provide feedback and review if this can be improved in any way.

public class queueUsingStack {

    static Stack<Integer> s1 = new Stack();
    static Stack<Integer> s2 = new Stack();

    public static void enqueue(int element){
        s1.push(element);
        System.out.println(element+ " inserted");
    }

    public static void dequeue(){
        if(s2.isEmpty()) {
            while (!s1.isEmpty()) {
                s2.push(s1.pop());
            }
        }
        System.out.println(s2.pop() + " removed");
    }

    public static void main(String args[]){
        enqueue(23);
        enqueue(223);
        enqueue(423);
        dequeue();
        dequeue();
        enqueue(231);
        enqueue(2399);
        enqueue(230);
        enqueue(2113);
        dequeue();
        enqueue(1223);
        enqueue(2300);
        dequeue();
    }
}
\$\endgroup\$
  • \$\begingroup\$ Is there a specific reason for doing it with a stack?? I see that your enqueue and dequeue works, but suppose I need to display my queue itself, how would I do that?? \$\endgroup\$ – leoOrion Jun 14 '16 at 3:35
  • \$\begingroup\$ Just a random problem I found to solve. \$\endgroup\$ – Neelesh Salian Jun 14 '16 at 3:38
  • \$\begingroup\$ How would you display the queue?? I happened to find this question. I think it may help you..-codereview.stackexchange.com/questions/54762/… \$\endgroup\$ – leoOrion Jun 14 '16 at 3:39
  • \$\begingroup\$ Thanks @leoOrion. However, that code keeps shuffling the stacks on every pop operation - which doesn't seem necessary unless the auxiliary stack is empty after filling it once. \$\endgroup\$ – Neelesh Salian Jun 14 '16 at 3:45
4
\$\begingroup\$

Use OOP

Everything in your code is static. While this is enough to make your code work, you should generally try to minimize the use of static variables and static methods and instead use classes and objects. Because everything in your code is static, you can't create a second QueueUsingStack since the stacks are global and do not differ per Queue.

Your code can easily be rewritten to make use of OOP:

public class QueueUsingStack {

    private Stack<Integer> s1 = new Stack<>();
    private Stack<Integer> s2 = new Stack<>();

    public void enqueue(int element){
        s1.push(element);
        System.out.println(element + " inserted");
    }

    public void dequeue(){
        if(s2.isEmpty()) {
            while (!s1.isEmpty()) {
                s2.push(s1.pop());
            }
        }
        System.out.println(s2.pop() + " removed");
    }

    public static void main(String args[]) {
        QueueUsingStack q = new QueueUsingStack(); 
        q.enqueue(23);
        q.enqueue(223);
        q.enqueue(423);
        q.dequeue();
        ...
    }
}

Use Generics

Your Queue can only hold Integers. If you want your Queue to support other objects as well, you should use Java Generics.

Rewriting your code to make use of Generics would look like this:

public class QueueUsingStack<T> {

    private Stack<T> s1 = new Stack<>();
    private Stack<T> s2 = new Stack<>();

    public void enqueue(T element){
        s1.push(element);
        System.out.println(element + " inserted");
    }

    public void dequeue(){
        if(s2.isEmpty()) {
            while (!s1.isEmpty()) {
                s2.push(s1.pop());
            }
        }
        System.out.println(s2.pop() + " removed");
    }

    public static void main(String args[]) {
        QueueUsingStack<Integer> integerQueue = new QueueUsingStack<>(); 
        integerQueue.enqueue(23);
        integerQueue.enqueue(223);
        integerQueue.enqueue(423);
        integerQueue.dequeue();
        ...

        QueueUsingStack<String> stringQueue = new QueueUsingStack<>();
        stringQueue.enqueue("string");
    }
}

Furthermore, you're omitting the type when creating the stacks in your code.

Stack<Integer> s1 = new Stack();

should be:

Stack<Integer> s1 = new Stack<Integer>();

If you're you using Java 7 and up you can also make use of the Diamond operator:

Stack<Integer> s1 = new Stack<>();

Naming

Class names should start with an upper case letter in Java.

\$\endgroup\$
2
\$\begingroup\$

The posted class is useful for only one thing: printing stats about items added and removed from a queue, in FIFO order. But it is not a queue. If it was a queue, I could add elements to it, and get elements from it for processing.

In short, change the dequeue method to return the item, instead of just printing.

It's an implementation detail that the queue uses stacks to store data. This detail should be hidden from the outside, by making the stacks private.

It's not normal for a queue to print its actions, I suggest to remove the print statements.

A main method also doesn't belong in a queue. Code that demonstrates the usage of some class can go in a QueueDemo class, for example.

I understand that you don't intend to use this code in real life, it's just an exercise. Btw it's also a classic interview question. And even though the algorithm is spot-on, this won't fly at an interview. As the saying goes, you play how you practice. It's good to at least pretend to be a bit serious, and if you implement a queue, even if it's in some tortured way, do follow the common interface, and use OOP well.

\$\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.