4
\$\begingroup\$

I need your expert opinion! In fact, for a Leetcode Problem, I had to code a queue using 2 stacks. It's shown just below and functions well.

It is a pretty simple code with peek, pop, push, etc. functions, quite classic. But this code does not please me completely since I coalesce the optionals with a default value, taken out of my hat, to avoid forced unwrapping.

Do you see a more professional way to write this?

class MyQueue {

private var stack1: [Int]
private var stack2: [Int]
private var front: Int?

init() {
  stack1 = []
  stack2 = []
}

func push(_ x: Int) {
  if stack1.isEmpty {
    front = x
  }
  stack1.append(x)
}

func pop() -> Int {
  if stack2.isEmpty {
    while !stack1.isEmpty {
      stack2.append(stack1.popLast() ?? -1)
    }
  }
  return stack2.popLast() ?? -1
}

func peek() -> Int {
  if !stack2.isEmpty {
    return stack2.last ?? -1
  }

  return front ?? -1
}

func empty() -> Bool {
  return stack1.isEmpty && stack2.isEmpty
    }
}
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

Don't use “magic” values (in your case: -1) to indicate the absence of a value. Swift has the Optional type exactly for that purpose. The queue methods should be declared as

func push(_ x: Int)
func pop() -> Int?
func peek() -> Int?
func empty() -> Bool

and nil is returned if the queue is empty.

There is nothing special about the Int type here. It is simple to make the queue type generic

class MyQueue<Element> {

    func push(_ x: Element)
    func pop() -> Element?
    func peek() -> Element?
    func empty() -> Bool
}

and now you can use it with arbitrary types. For example

let queue = MyQueue<String>()
queue.push("foo")

There is no need for a dedicated front element, and the code becomes simpler without it.

Moving the elements from the first to the second stack in func pop() can be simplified to

while let elem = stack1.popLast() {
    stack2.append(elem)
}

with a single test instead of two per iteration. Or even simpler:

stack2 = stack1.reversed()
stack1.removeAll()

(If your queues become really large then you might want to check which of the above methods is faster.)

If a function body consists of a single expression then the return keyword can be omitted.

Putting it all together, we get

class MyQueue<Element> {
    
    private var stack1: [Element]
    private var stack2: [Element]
    
    init() {
        stack1 = []
        stack2 = []
    }
    
    func push(_ x: Element) {
        stack1.append(x)
    }
    
    func pop() -> Element? {
        if stack2.isEmpty {
            stack2 = stack1.reversed()
            stack1.removeAll()
        }
        return stack2.popLast()
    }
    
    func peek() -> Element? {
        stack2.last ?? stack1.first
    }
    
    func empty() -> Bool {
        stack1.isEmpty && stack2.isEmpty
    }
}

which is shorter and simpler.

Finally, naming:

  • I had to inspect the code in order to understand that what you have implemented is a “first in, first out” queue. This would be more obvious to a user of your code (or to you in one year) with a more specific class name, such as FIFO or FIFOQueue.

  • Typical names for adding something to a FIFO and retrieving it are enqueue and dequeue.

  • Swift collection types typically use “isEmpty” as the name of a property which indicates whether the collection contains elements or not:

    var isEmpty: Bool {
        stack1.isEmpty && stack2.isEmpty
    }
    
\$\endgroup\$
5
  • \$\begingroup\$ I took note of your improvements. The disappearance of the variable "front" has effectively lightened the code. Leetcode is now giving me 0ms execution time and my submission is beating 100% of submissions. However, for this exercise, Leetcode requires that the functions return Int types and refuse Int?. So I have two versions of my Queue: the version posted on Leetcode and the improved version that I will use, thanks to you, in my projects. Finally, I learned that a stack is a LIFO data structure while queue is a FIFO one. Thus the naming seemed OK... Thank you so much for your help! \$\endgroup\$
    – Thomas H.
    Nov 23, 2022 at 16:53
  • 1
    \$\begingroup\$ @ThomasH. I'm not sure what is the swift way of doing things, but I usually think of calling peek/pop on an empty queue/stack as being error, it should throw, rather then returning any special value. Callers are responsible for calling isEmpty first, if they don't do that the client code is broken and it makes sense to throw error. Returning special value means that for the sake of satisfying those who use the queue wrong, you complicate life for those who use it correctly. \$\endgroup\$
    – slepic
    Nov 24, 2022 at 5:38
  • \$\begingroup\$ Hence I totally understand why leetcode doesn't allow optional return type on those methods... They also expect you to throw errors... \$\endgroup\$
    – slepic
    Nov 24, 2022 at 5:45
  • \$\begingroup\$ In fact, peek and pop aren't even expected to call isEmpty themselves. They should just execute whatever logic they need assuming it is not empty and the execution will just fail at some point (in this case specifically when trying to return value but having none) if the assumption is wrong. This is called undefined behaviour and it means anything (including nothing) may happen. It's like division by zero, it's not zero, it's not infinity, it's not nil, it's nothing, it's undefined, its error. \$\endgroup\$
    – slepic
    Nov 24, 2022 at 6:03
  • 1
    \$\begingroup\$ @slepic: If this is for a LeetCode problem and the task is to “write a function pop() that returns the first element of the queue, or -1 if the queue is empty” then there is not much one can do. The Swift collection methods first, last, popLast etc return nil if the collection is empty, that's why I suggested the same behaviour here. But LeetCode problems are often not posed in a “Swiftly way” because solutions can be submitted in many programming languages. \$\endgroup\$
    – Martin R
    Nov 24, 2022 at 6:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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