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.