search

fnoori / Vivyd-Calculator

0 stars 0 forks source link

Complicated bracket bug #82

Open amairJD opened 7 years ago

amairJD commented 7 years ago

Say user enters "(5(" When user tries to calculate, obviously there is an error. But check this out. Before calculation, numleftbracket is 2, and numRightbracket is 0. If I calculate, get error, and THEN press right bracket, numRightbracket jumps for 0 to 3! Oh lord.

fnoori commented 7 years ago 
                if (closeBracket != openBracket)
                    throw new BadBracketsException();

This piece of code is not doing anything Note that immediately before this piece of code, equalButtonLogic is being called recursively. Since it's being called recursively, whatever code that comes after the recursive call will never be executed.

I think I know where the (5( bug is occurring, will update soon

fnoori commented 7 years ago

Alright, it wasn't displaying the new "updated" expressionToEvaluate after the brackets were mismatched. Something unfortunate happens here. Since we are implementing the "smart" bracket insertion, for cases like this:

(5( -- presses "=" (5( will become (5()) -- this happens because the equalLogic is attempting to "fix" the bracket mismatch. I can think of 1 way to prevent this, will look into it more to figure out a better way

fnoori commented 7 years ago

I see that the in "Calc+" our model calculator ;). Does something similar to what ours does now

amairJD commented 7 years ago 
        if (closeBracket != openBracket)
                    throw new BadBracketsException();

You can delete that if you like, but make sure to get rid of the catch block for as well BadBracketsException().

Good luck with trying to fix, let me know if I can help

fnoori commented 7 years ago

I've implemented the way Calc+ does it Basically if "(5(" is entered, an error is thrown and "(5())" is displayed in answerView There isn't another way around this, unless we decide to remove "smart" bracket fixes, which I think is more beneficial in a lot of other circumstances

amairJD commented 7 years ago

Coolio, A1.

Yeah let's keep smart bracket fixing. Super useful it is.

But let's keep this issues open just in case we get an idea in future of how to make this even better :D