I would like to get feedback on my code for Stack implementation written in scala. What can be improved to make it more like scala code. What important helper functions am i missing?
class StackDS[A] {
private var list: List[A] = List[A]()
def push(ele: A) =
list = ele :: list
def pop = {
val pop = list.head
list = list.tail
pop
}
def size: Int = {
list.size
}
}
1 Answer 1
layout - The code is clear and easy to read but I find your use of blank lines a bit excessive.
braces - Both the push() and size methods consists of a single line of code so surrounding braces, {..}, are optional. You drop them in one and keep them in the other. Inconsistent.
types - The size method declares its return type but push() and pop don't. It's usually a good idea to include them in all but the smallest, simplest method definitions.
mutation - The mutating collection, list, is kept local and private, which is good. I might argue that it's better to keep a mutable collection, such as Buffer for example, in a val variable instead of keeping an immutable collection, List, in a var variable.
mutating methods - For methods with no arguments that mutate an internal state, such as pop does, the Scala Style Guide recommends declaring the method with empty parentheses, def pop() ..., as an indicator to the end user.
safety - The code will throw an exception if you try to pop an empty stack. That may be as you want it. I prefer methods that return Option[A] instead of throwing.
convenience - If I was trying to use StackDS in my code, I'd want to know why stacks have to be created empty. Why can't I create a populated stack? e.g. new StackDS(8,4,11) It would require a small modification to the existing code.
What I think your code clearly demonstrates is that a simple List does most of what we need from a Stack. So even though the Standard Library includes a full featured Stack implementation, it's not often used.