Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification in the docs #17

Open
jasonmhite opened this issue May 4, 2018 · 2 comments
Open

Clarification in the docs #17

jasonmhite opened this issue May 4, 2018 · 2 comments

Comments

@jasonmhite
Copy link

jasonmhite commented May 4, 2018

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.

@soveran
Copy link
Owner

soveran commented May 4, 2018

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 :-)

@jasonmhite
Copy link
Author

jasonmhite commented May 4, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants