My RPN calculator code review?

Anonymouse zorael at gmail.com
Sat Jul 18 10:33:23 UTC 2020


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.


More information about the Digitalmars-d-learn mailing list