Re: My RPN calculator code review?

[Dukc via Digitalmars-d-learn]

On Friday, 17 July 2020 at 21:37:46 UTC, AB wrote:
I'd appreciate your opinions regarding style, mistakes/code 
smell/bad practice. Thank you.


In a project this small, implementability (meaning, ease of 
writing) is really the main guideline, readability is a 
non-issue. When your codebase hits a few hundred lines in size 
you should start to gradually pay more attention to 
readability/scalablity.


In fact, things that are recommended in 2000-line projects may 
even be antipatterns in 100-line projects. For example, unit 
testing greatly reduces bugs, so it's highly recommended in 
general. But if your project is so small that normal manual 
testing covers most everything anyway, unit testing is just 
needless bloat.


A github gist might be better when doing style reviews, because 
the forum snippets tend to have wandering indentation and line 
wrapping.

Re: My RPN calculator code review?

[AB via Digitalmars-d-learn]

On Saturday, 18 July 2020 at 10:33:23 UTC, Anonymouse wrote:
I'm not happy about the looping and allocating replacements of 
spaced_args,


Actually, I got rid of that code entirely in order to support 
negative values:


-5 -6 *
== 30

Re: My RPN calculator code review?

[Anonymouse via Digitalmars-d-learn]

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.

My RPN calculator code review?

[AB via Digitalmars-d-learn]

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.


import std.array;
import std.conv;
import std.stdio;

void main(string[] args)
{
if (args.length == 1)
{
writeln("usage examples:\n\t", args[0], " 5 3 + 2 
*\n\t", args[0],

" 12.1 11 /");
return;
}

immutable string valid_ops = "+-*/";
string spaced_args = args[1 .. $].join(" ");

// this is done to correctly handle "messy" inputs such 
as "5 3 2*+"

foreach (o; valid_ops)
{
spaced_args = spaced_args.replace(o, " " ~ o ~ " ");
}

real[] stack;

foreach (op; spaced_args.split)
{
LabeledSwitch:
switch (op) // operator or operand?
{
static foreach (o; valid_ops)
{
case [o]:
stack = stack[0 .. $ - 2] ~
mixin("stack[$ - 2]" ~ o ~ "stack[$ - 
1]");

break LabeledSwitch;
}
default:
// assume it's an operand, a real number
stack ~= to!real(op);
break;
}
}

writeln(stack);
}