BugFix3301: Precedence of % (mod) is higher than * and /

[nkumawat34]

Added the BugFix of Issue #3301

This PR addresses the problem where the precedence of the % (modulo) operator was incorrectly treated as having higher precedence than * and /. The fix ensures that % now has the same precedence as * and /, resolving the issue caused by the functional addition for % that led to incorrect operator precedence.

[josdejong]

THanks @nkumawat34 , impressive! I thought this would be tricky to fix.

Two remarks:

1. Can you update the Precedence table: https://github.com/josdejong/mathjs/blob/develop/docs/expressions/syntax.md#precedence
2. Can you add more unit tests to check the precedence of % and mod vs the four other operators * / .* and ./ ? (precedence should be equal now)

[nkumawat34]

Please check now I updated the PR

[josdejong]

Thanks, that's a thorough est of tests 😎

One idea: how about changing the parseAndEval approach to the following:

function parseAndStringify(expr) {
  return math.parse(expr).toString({parenthesis: 'all'})
}

// ...
assert.strictEqual(parseAndStringify('10 % 3 * 2'), '(10 % 3) * 2')

Then there is no need for the explanatory comments like // Expected as (10 % 3) * 2 = 1 * 2 = 2? We would lose checking the actual outcome though. What do you think? (I'm ok with both, it's just an idea)

[josdejong]

Thinking about it, we could do something like this:

function assertParseAndEval(expr, expectedString, expectedResult) {
  const node = parse(expr)
  assert.strictEqual(node.toString({ parenthesis: 'all' }), expectedStr)
  approxEqual(node.evaluate(), expectedResult)
}

// ...

assertParseAndEval('10 % 3 * 2', '(10 % 3) * 2', 2)

😎

EDIT: I see there is already a helper function parseAndStringifyWithParens that is use to test implicit multiplication.

[nkumawat34]

should I have to use parseAndStringifyWithParens or not ?

[josdejong]

I'm ok with any of the three options: leave as is, create and use assertParseAndEval, or use parseAndStringifyWithParens. I have a slight preference for using parseAndStringifyWithParens. What do you think?

[nkumawat34]

Ok I am going with this parseAndStringifyWithParens.I think it will be better.

[nkumawat34]

done

[josdejong]

Thanks for the quick updates 👌, merging your PR now in the v14 branch.

[josdejong]

Published now in v14.0.0, thanks again Neeraj!