search

silentmatt / expr-eval

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

Bring back the 'if' function #210

Closed jpike88 closed 5 years ago

jpike88 commented 5 years ago

Just updated expr-eval to find removal of the 'if' function has broken heaps of stuff. There was no need to remove the if term. Forcing people to use ternary operators can make certain code harder to read, not easier. Please bring it back.

Backwards compatibility should be priority number one in a library like this. People shouldn't need to update and exhaustively read changelogs on the offchance that all the strings they've been storing and building are rendered obsolete.

silentmatt commented 5 years ago

I'm not sure I agree on the readability, but I also realize that's just my opinion. I see your point about backward compatibility, and agree to a point. I generally try to keep things backwards-compatible as much as possible/reasonable, with new features between major releases being opt-in if they could break things.

The problem with the if function, and the reason I removed it is that, being a function, and not a built-in operator, it always evaluates both the if and else parts. That's fine in a lot of cases as long as there are no side effects, but it could be a problem if you're not expecting it.

Having said all that though, when I decided to remove it, I thought I had a note in the README about it being deprecated, but apparently I was imagining that, and it's definitely not fair to pull it out without warning. So for now at least, I'll add it back, with a note about the potential issues.

jpike88 commented 5 years ago

You're a legend, thanks.

I think the risk here lies in the fact that there is no type checking or warnings otherwise either during dev or production that such a change is occurring.

Also, I actually need the if and else to be evaluated, as I'm nesting the calls. That's what you meant by evaluating both right?

I also ask that you keep it in indefinitely, as I have a buttload of strings stored in our database that utilize the if call, and rely heavily on this library to be as stable as possible in that sense.

I might need to write some tests to detect a change like that if you can't guarantee that