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

Support percentage operator in parser #53

Open
titoworlddev opened this issue May 4, 2021 · 9 comments
Open

Support percentage operator in parser #53

titoworlddev opened this issue May 4, 2021 · 9 comments
Labels
enhancement Indicates new feature requests

Comments

@titoworlddev
Copy link

titoworlddev commented May 4, 2021

It just doesn't work right, I have tried to do the calculation of various percentages but it doesn't work right, it gives wrong result.
For example if I put something as simple as:
'8%9'.interpret();
the output is: 8
when it should be: 0.72

If you could fix it please because this gives me a lot of problems and I need to make a calculator that is already ready and that error will not let me finish it.

@fkleon
Copy link
Owner

fkleon commented May 5, 2021

Hi @Hekhy, the % symbol denotes the Modulo operator, not the percentage operator.

You need to use decimal notation to perform percentage calculations, see #28 and #50

@fkleon fkleon added the duplicate Indicates similar issues or pull requests label May 5, 2021
@titoworlddev
Copy link
Author

Well I have had to do my method and every time I press the button with the symbol "%", "/ (100) *" is added, then I apply a format so that "/ (100) *" becomes "% "but only in sight.
It is the only way that it works well, since if you make that when giving "%" the last number is divided by 100 and you get the result (for example 0.25) it gives problems, the only way is that you add it manually but I want to give people that facility of not having to do it and it becomes automatic.
It would be nice if you implemented it, even so thanks, the package is very good.

@fkleon fkleon added enhancement Indicates new feature requests and removed duplicate Indicates similar issues or pull requests labels May 6, 2021
@fkleon fkleon changed the title The "%" operator does not work well Support percentage operator in parser May 6, 2021
@fkleon
Copy link
Owner

fkleon commented May 6, 2021

Good to hear that you found a workaround. Will consider adding support for this to the parser!

@jvinai
Copy link

jvinai commented Sep 13, 2021

Any news about adding the percent operator?

@titoworlddev
Copy link
Author

Any news about adding the percent operator?

It seems that nothing yet, it has been a long time since I gave my option, that although it is not the best it is how I fix it, if it helps you let me know and I put my code although it is more or less explained above and if not I hope the creator is current early.

@Ruwasoft
Copy link

is any update for this issue ?

@GregoryConrad
Copy link

GregoryConrad commented Sep 20, 2022

Here is my workaround (the key here is the regex with the replaceAllMapped):

extension Expression on String {
  double evaluate(Map<String, double> variables) {
    final cm = ContextModel();
    for (final variable in variables.entries) {
      cm.bindVariableName(variable.key, Number(variable.value));
    }
    final percentsReplaced = replaceAllMapped(
      RegExp(r'(\d+(\.\d*)?)%'),
      (match) => '(${match.group(1)!}/100)',
    );
    return Parser().parse(percentsReplaced).evaluate(EvaluationType.REAL, cm);
  }
}

However, this will break support for modulo unless you have a space before the % and the first number; use with caution.

@fkleon I was taking a look at the factorial support PR (#58) to see how feasible it would be for me to make a PR for basic percentage support (supporting the equivalent complexity of 80% * 100 => 80--a simple substitution of the val% with val/100). However, the keywords map in Lexer proves to be problematic as it maps a single keyword to a single token, which is not always the case (as seen here with modulo and percent). Maybe it would be worth it to switch to a more complex solution/algorithm? I noticed a few hacks in the parser already to make it work, notably the e and ceil conflict amongst at least one more documented in code. Perhaps a CFG could work nicely?
Edit: it looks like https://pub.dev/packages/petitparser would work nicely.

@fkleon
Copy link
Owner

fkleon commented Nov 3, 2022

HI @GregoryConrad - you're right about the shortcomings of the current lexer. I've looked into alternative implementations but never finished them. Busy elsewhere so struggling to find any time to work on this library!

Coming from a programming background I find the modulo operator to be a more "natural" fit for %, but can understand that people want to use a percentage operator. However it seems obvious to me that you can simply use the decimal representation in your expressions so I haven't prioritised this. However it's a popular request for sure!

@titoworlddev
Copy link
Author

titoworlddev commented Nov 5, 2022

@fkleon
You're right about simply using the decimal representation. But the fact of creating this thread, for me, was because I was creating a calculator that is for everyone. This means that there are people who don't like having to think, they just put the % operator and that's it, so that's why I wanted to give it that functionality.

In the end, as I commented some time ago in a comment, my solution was to divide by 100, to get that decimal result and be able to do the same thing I want (for example: 25/100 => 0.25), and then simply when you hit the operator % replacement 25/100 for the same (25/100 => 25%).

I know it's a mess, but if you look at all calculators have this and that's why I said, since they are for everyone there are people who just don't want to think about how to do it.

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

No branches or pull requests

5 participants