On Friday, 17 July 2020 at 21:37:46 UTC, AB wrote:
Hello, inspired by the "Tiny RPN calculator" example seen on
the dlang homepage, I wanted to create my own version.
I'd appreciate your opinions regarding style, mistakes/code
smell/bad practice. Thank you.
I generally don't know what I'm talking about, but nothing stands
out as outright wrong. Style is unique from person to person.
valid_ops is a compile-time constant and can be made an enum.
I'm not happy about the looping and allocating replacements of
spaced_args, but it's an easy solution where alternatives quickly
become tricky or involve regular expressions (something like
`spaced_args.matchAll("[0-9]+|[+*/-]".regex)`). Regex is neat but
heavy.
You can use std.algorithm.iteration.splitter instead of
std.array.split to lazily foreach spaced_args.
I don't know enough to say if `case [o]` will allocate at
runtime. If so, it could be replaced with `case to!string(o)`,
but maybe the compiler is smart enough to not consider [o] a
runtime literal. For a program this short it doesn't really
matter, but for a longer one it might be worth looking up.
You're not checking for array size before slicing stack, and as
such the program range errors out on operator-operand count
mismatch.
The rest is micro-optimisation and nit-picking. If valid_ops is
an enum, you could foreach over it as an AliasSeq with a normal
foreach (`foreach (o; aliasSeqOf!valid_ops)`; see std.meta). You
could use std.array.Appender instead of appending to a real[].
etc.
