silentmatt / expr-eval

Mathematical expression evaluator in JavaScript
http://silentmatt.com/javascript-expression-evaluator/
MIT License
1.19k stars 239 forks source link

Add statement separator #200

Closed eliot-akira closed 5 years ago

eliot-akira commented 5 years ago

This adds a statement separator ; as per #65.

A few notes on implementation:

eliot-akira commented 5 years ago

Additional notes, after some exploration:

By the way, using and exploring this library has been a joy. I hope I'm not bothering you too much with recent pull requests. I'm interested in working on array/list and function assignment next.


Off topic, but I created a web sandbox for expr-eval.

eliot-akira commented 5 years ago

I found a failing test case for my initial implementation: 2+(x=3;y=4;z=x*y)+5.

It was tough adding support for separators in nested expression. It passes all tests, but I'm not satisfied with the changes I made in the evaluator - I had to add an extra step for cases when a value is expected but an expression was received instead, like in the above test case.

eliot-akira commented 5 years ago

Aha, I got it. I added a new instruction type called IEXPREVAL which allows an expression to be evaluated later. This is a much cleaner, reliable and extensible solution.

silentmatt commented 5 years ago

Thanks for this! I definitely appreciate it especially since I haven't gotten to spend much time working on this lately. I'll take a deeper look at it when I get a chance and hopefully get this merged soon.

I think I agree with the choice to use ; instead of , since it makes parsing easier. And even though we only have expressions, and not statements, the idea is similar enough that I don't think it would be confusing.

I also love the sandbox. That's a great idea!

eliot-akira commented 5 years ago

Thank you for the feedback. I'm having fun with the code base, so there will probably be more pull requests coming. :) For sure, please take your time - and please feel free to not accept any pull requests that you think are beyond the scope of the library. I understand these are kind of experimental features still.

I've been using the npm "watch" script, together with the sandbox loading the development build. The easiest way I found is to create a symbolic link from dist/bundle.js to expr-eval.js in the sandbox.

So I run npm run watch for expr-eval, and in another terminal window/tab, npm run serve for the expr-eval-sandbox. It works well for making changes in the library and testing different expresssions to see the resulting tokens/instructions. In a third terminal tab, I regularly do npm run test.


I think I agree with the choice to use ; instead of , since it makes parsing easier. And even though we only have expressions, and not statements, the idea is similar enough that I don't think it would be confusing.

I did have a thought that maybe it should be called an "expression separator". It'd be simpler that way, to not add a new concept of a "statement" (which I guess is basically the same thing).

eliot-akira commented 5 years ago

For what it's worth, I looked up the definitions of expression and statement.

In many languages (e.g. C++), expressions may be ended with a semicolon (;) to turn the expression into an expression statement. This asks the implementation to evaluate the expression for its side-effects only and to disregard the result of the expression (e.g. "x+1;") unless it is a part of an expression statement that induces side-effects (e.g. "y=x+1;" or "func1(func2());").

I was glad to read the part about disregarding the result of the expression. The way I implemented it in the evaluate function was to pop the stack when a sentence ends.