hoaproject / Math

The Hoa\Math library.
https://hoa-project.net/
366 stars 37 forks source link

Decimals without a leading integer cause errors #57

Open BillBuilt opened 7 years ago

BillBuilt commented 7 years ago

Example: $expression = '.35 + .65'; $ast = $compiler->parse($expression); echo $visitor->visit($ast);

Result: Uncaught Hoa\Compiler\Exception\UnrecognizedToken, code: 0 Message: Unrecognized token "." at line 1 and column 1: .35 + .65 ↑ File: /web/htdocs/lmifirearms/vendor/hoa/compiler/Llk/Lexer.php Line: 1

BUT

$expression = '0.35 + 0.65'; $ast = $compiler->parse($expression); echo $visitor->visit($ast);

Works fine.

This is problematic if evaluating formulas entered by end users, which I am ;-). Thanks

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/45404152-decimals-without-a-leading-integer-cause-errors?utm_campaign=plugin&utm_content=tracker%2F894946&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F894946&utm_medium=issues&utm_source=github).
Hywan commented 7 years ago

Hello :-),

The syntax for real numbers must have an integer part, so something before .. This is expected. Would you like to be able to default to 0 for the integer part is msising?

BillBuilt commented 7 years ago

Hi. The problem I'm having is that I am using your library (which I love, btw) to safely parse expressions entered by end users so I have no real control over how they enter these types of numbers. So yes, if you could default it that way, that would be perfect. So if .743 is sent to your parser, it could convert it to 0.743 internally to avoid the error? Otherwise, I'll need to do some tricky pre-checking before passing it to your parser. Thanks again!

Hywan commented 7 years ago

Any @hoaproject/hoackers to try this? Maybe @Grummfy or @vonglasow?

stephpy commented 7 years ago

👍 IMO.

Looks pragmatic to support .1234 as 0.1234, even if it's not defined in any arithmetic rules. I don't see any possible conflict for this expression, so it would be a bonus.

vonglasow commented 7 years ago

I don't think it will be complicated to implement also so :+1:

Hywan commented 7 years ago

@vonglasow So, how would you tackle this problem? There is many ways to do so, can you expose yours :-)?

BillBuilt commented 7 years ago

For now, I'm passing user entered expressions to the following function to "clean" them first. So far, so good, but I'm guessing you would have better ways of achieving this 😃 I just know at some point, someone is going to get something by this simple little regexp but until then, all is good. Thanks for a great lib. So far, yours is the only one I have tried that actually works and has built in support for floor().

function clean_math_expr($expression=null) {
    return str_replace(' ', '', preg_replace("/(\D|^)\.(?=\d)/", '$1 0.', $expression));
}
Pierozi commented 7 years ago

Like it seems all calculator (physically or virtual) allow to begin by dot for decimal it should be a good idea to follow the same common usage.

Hywan commented 7 years ago

@vonglasow There is several strategies possible. I would like to hear yours before starting a PR :-).