sklose / NCalc2

expression evaluator for .NET with built-in compiler
MIT License
170 stars 58 forks source link

Why calculating with boolean values is prohibited? #15

Closed lukas-ais closed 6 years ago

lukas-ais commented 6 years ago

In the current version calculating with boolean values is prohibited. There is a Unit Test explicitly preventing such calculation: Fixtures.ShouldDisplayErrorIfUncompatibleTypes.

We need such calculation. Would you accept a PR to implement this?

sklose commented 6 years ago

I don't quite follow, what would be the result of this expression?

        [Fact]
        public void ShouldDisplayErrorIfUncompatibleTypes()
        {
            var e = new Expression("(a > b) + 10");
            e.Parameters["a"] = 1;
            e.Parameters["b"] = 2;
            Assert.Throws<InvalidOperationException>(() => e.Evaluate());
        }
lukas-ais commented 6 years ago

(a > b) is the boolean value. It will create either 'true' or 'false'. According to the C style they can be mapped to '1' or '0'. So, this expression will result in either '11' or '10'. With the given parameters "a" and "b" the expression will return '10'.

sklose commented 6 years ago

I don't feel like that's the desired behavior and probably confusing for most people. If we add support for this, then it should probably be off by default and opt-in via the EvaluateOptions.

cd21h commented 6 years ago

Adding or subtracting boolean values does not make any sense, consider (true + true) * 10 . If such hack is required, I would implement conversion somewhere else

lukas-ais commented 6 years ago

I've created PR #16. There I added new option (and refactored existing flag values) make this feature opt-in.

@shatl, your example really does not looks reasonable. But I'm work on a project with high amount of existing formulas including such strange formulas.

Currently we have our own fork of the original (codeplex) code. We would like to drop our own fork and work together on a single code base. So, I think it would be useful add all required features with optional flags, isn't?