Adding support for NaN and Infinity

[lipchev]

As discussed in #26

• NaN represented by {0/0}
• PositiveInfinity represented by {1/0}
• NegativeInfinity represented by {-1/0}
• Zero is now represented by {0/1}
• default(Fraction) results in {0/1} (with FractionState.Unknown)
• Supporting all types of non-normalized fractions such as: {10/0} (+Infinity), {0,-10} (Zero), {-10/0} (-Infinity)
• Benchmark results show an all-around performance improvement * (note these aren't so much related to the introduction of NaN, but more as a result of the more granular handling of the edge-cases)

I ended up not changing the FractionState enum, instead it is being check for when returning the default Denominator

Breaking changes:

• Fraction.Zero.Denominator now returns 1
• no exceptions during construction
• since it is now possible to construct fractions with 0 in the Denominator - functions such as ToInt64() or ToDecimal() would throw a DivideByZeroException
• according to the same logic Fraction.NaN.ToString("n") would also throw a DivideByZeroException
• I'm not sure if before it was possible to parse "0/-10" as a non-normalized Zero, and then have it ToString() back to it's original value "0/-10"

[lipchev]

There's a lot to review, I'm sorry- I didn't mean to change every single file but there were a lot of case to check for especially when it comes to the non-normalized fractions. I initially didn't consider having them supported for the positive/negative infinity, but as i reached the string <-> Fraction <-> string scenarios I thought, whatever- let's have all possible "a/b" cases covered..

I've left (myself) a few TODOs with possible shortcuts to take, but wanted to have something to test them against (the benchmarks that I uploaded with this release).

While updating/testing the code, I also noticed that there isn't a consistent rule regarding the FractionState of operations such as Multiply/Divide/Reciprocal - most of the time it appears we're forcing a normalization, but not always..

[danm-de]

Thanks for the pull-request.

Unfortunately, due to time constraints, I need a little longer for the code review this time (a few days).

I still have a quick question:

Is the following condition always met?

Fraction.Zero == default(Fraction) 

[lipchev]

Thanks for the pull-request.

Unfortunately, due to time constraints, I need a little longer for the code review this time (a few days).

I still have a quick question:

Is the following condition always met?

Fraction.Zero == default(Fraction) 

Yes, that should evaluate as true

[lipchev]

I've left (myself) a few TODOs with possible shortcuts to take, but wanted to have something to test them against (the benchmarks that I uploaded with this release).

I went over these in my last commit, and added (after substantial amount of benchmarking) the required edge-case-testing in order to minimize the number of BigInteger allocations (most often due to large multiplications).
I've specifically tested the effect on using the GCD to reduce the size of the terms added/multiplied- this was only hurting the performance, up to a certain size, after which having two very-large numbers multiplied is significantly more expensive than figuring out their GDC and doing all the extra divisions etc. There is probably some formula that can be determined here, but I didn't go as far. I just tested one against the other for the different cases and went with that:

• CompareTo: since there are no further allocations (no additions/multiplications) using the GCD was always slower (i've removed it)
• Add: here the results were mixed- mostly better without the pre-normalization, except for the very large cases - where the difference was quite significant (i've left it in)
• Remainder: same as with the addition, only here there are even less cases where the GCD is worth it (i've left it in)

It takes a lot more ifs and elses, but even operations such as Multiply/Divide/Pow can benefit from not doing two multiplications (when one is expected to return 0):

    // we want to skip the multiplications if we know the result is going to be 0/b or a/0

Note that these improvements could be applied as "non-breaking-optimizations", if we're committed on having a final/stable release of the v7... We're talking more than 20%-30% improvements on most benchmarks..

[danm-de]

Since the changes are very extensive, I have integrated your GIT branch under https://github.com/danm-de/Fractions/tree/lipchev-support-nan-and-infnity

I will apply my changes directly there, rather than using GITHUB's comment function. Reason: many changes have an impact on many (sub)functions and unit tests.

I'm currently trying your suggestion regarding the Nullable<BigInteger> for the denominator. The special handling for the default(Fraction) case is too complicated for me or I find the safety tests too difficult to understand 😄

[danm-de]

I've finished the code review and have published my suggested changes directly to the following branch:

https://github.com/danm-de/Fractions/tree/lipchev-support-nan-and-infnity

It would be nice if you could take another look.

The main changes are:

• Auto-Properties for the win (backing fields removed if possible)
• Denominator is now nullable
• Correction of the ToString(..) method

[danm-de]

I've finished the code review and have published my suggested changes directly to the following branch:

https://github.com/danm-de/Fractions/tree/lipchev-support-nan-and-infnity

It would be nice if you could take another look.

The main changes are:

* Auto-Properties for the win (backing fields removed if possible)

* Denominator is now nullable

* Correction of the ToString(..) method

Oh, I may introduced a problem with TryParse.