clac icon indicating copy to clipboard operation
clac copied to clipboard

Clarification in the docs

Open jasonmhite opened this issue 7 years ago • 2 comments

Hello again. While I was working on the stuff in #16 I noticed there is a point of confusion in the docs. Under the "commands" section it says "When a command requires an argument, it pops a value from the stack. If the stack is empty, a zero is provided instead."

This is not true for most of the commands; this is the internal behavior when pop is called on an empty stack, but most commands don't trigger that since they are guarded by if (count(s0) > ..) and the observed behavior is instead that the command becomes a noop. It is true for others, e.g. sum. This makes things a bit confusing and could probably stand to be clarified (or made consistent in the code if you prefer).

Again, nice project. It's been fun poking around and I appreciate how clean the code is.

jasonmhite avatar May 04 '18 03:05 jasonmhite

You are absolutely right, I think we should definitely clarify that paragraph and perhaps mention in a succint way what's the behavior for each group of commands. Maybe writing a note for each and every command will make the it too verbose, so we will have to think about how to redact it best. I'm glad you took the time to read the documentation carefully, thanks for spotting this :-)

soveran avatar May 04 '18 06:05 soveran

An easy and consistent solution would be to simply add the guard by count to the functions that don't have it right now, so that they also become no-ops. I mean something like changing

else if (!strcasecmp(word, "sum")) {
	push(s0, add(s0, count(s0)));
}

to

else if (!strcasecmp(word, "sum")) {
    if (count(s0) > 0) {
	push(s0, add(s0, count(s0)));
    }
}

This would change the behavior to make sum (and the others) a noop instead of default to zero, but it'd be consistent and that behavior seems intuitive to me. Not sure if that's what you want, but it's what I would suggest.

jasonmhite avatar May 04 '18 15:05 jasonmhite