Closed arnaudroques closed 3 years ago
Hi, thank you very much for contributing. I think, this would be an interesting feature, as long as there is backward compatibility for existing extensions, like custom functions and operators. It would be not so nice, if the users who have added custom operators and functions were forced to change their code. I would say, as long as all existing unit tests will run without the need for changing them, the change is ok.
It would be not so nice, if the users who have added custom operators and functions were forced to change their code.
I did not think about this... Unfortunately, yes, users will be forced to change their code. I understand that this is a concern to you. However, the change is really simple. They will have to add a cast like this:
addOperator(new Operator("+", OPERATOR_PRECEDENCE_ADDITIVE, true) {
@Override
public Number eval(Number nv1, Number nv2) {
BigDecimal v1 = (BigDecimal) nv1;
BigDecimal v2 = (BigDecimal) nv2;
assertNotNull(v1, v2);
return v1.add(v2, mc);
}
});
I agree that this is annoying, but the change is really easy to do.
Furthermore, working on Number
instead of BigDecimal
would for example also allow extensions managing https://en.wikipedia.org/wiki/Complex_number (if someone defines a new class extending Number
).
I've cloned your repository so that you can have a look at the result code. https://github.com/plantuml/EvalEx/commit/60033168c3eb237eed99420b7c1d132528c9fa10 Unit tests are all running ok (after code change).
This change would definitively need a major version number (so you would switch to EvalEx 3.0). As creator of EvalEx the decision is yours, but I suggest that you create a https://doodle.com about this kind of change to see if your users think that this code change is acceptable to them to allow nice future enhancements within EvalEx. :-)
@arnaudroques BigDecimal
is not final
, any reason you couldn't base NString
on that?
I was just looking at the BigDecimal
source, and none of its methods are final
either. So technically, we could provide a custom BigDecimalString
class which extends BigDecimal
and already implements all the required logic, like add
internal. So when you get the result back, you just need to check whether it is the "special" BigDecimal
or not for displaying the results.
Ok, I've not seen that BigDecimal
was not final
.
So I remove my first try and commit another one with a BigString
that extends BigDecimal
.
So no impact on existing custom operators.
The Snipset4
class shows that it works, unit test are running ok.
https://github.com/plantuml/EvalEx
However, because I was not sure about the best way to do, I've created a BIG_STRING
token type which should probably be merged with STRINGPARAM
. I've also temporary choosen simple quote to not clash with double quotes used for STRINGPARAM
.
Any idea how can I merge those two concepts ?
First thanks for your library, it is very well designed.
For my own purpose (I am working on https://github.com/plantuml/plantuml ) I am looking for a pure Java expression evaluator that would evaluate simple mathematical, boolean and string expression. So I took your code, and I've made some slight modification just to see if I would be able to extend it to have full string support. I made something quick & dirty. I first create a class:
Then, since BigDecimal extends Number, I basically change your code and put Number instead of BigDecimal to a lot of place. For example:
Somewhere in Expression.java, I now have:
Thanks to this, I have now the following working !
I understand that your library has not been designed to handle String, but I think it could easily manage them (maybe with something less dirty that I have quickly tried).
So before going further, I would like to know if you would be interested by such a move. If you are, we could work together to add this feature to EvalEx (with something cleaner that this Number/NString trick). This way, other people could use EvalEx to manage string and I could use your library directly in PlantUML. Any thought ?