patterncsharpMinor
Rational number implementation
Viewed 0 times
implementationnumberrational
Problem
I've been recently working on a custom rational number implementation. Due to not very interesting reasons, using this implementation is not an option.
I'd appreciate comments of anyone who has the time to review the code. The posted code is free to use, so you can make good use of it should you want/need to.
The implementation is basically finished except the
I'm particularly interested in the
```
using System;
using System.Diagnostics;
using System.Globalization;
using System.Numerics;
namespace MathSuite.Core.Numeric
{
[Serializable]
public struct Rational : IFormattable, IEquatable, IComparable, IComparable
{
#region Static fields
const string positiveInfinityLiteral = "Infinity";
const string negativeInfinityLiteral = "-Infinity";
const string naNLiteral = "NaN";
const int fromDoubleMaxIterations = 25;
static readonly Rational one = new Rational(1);
static readonly Rational zero = new Rational(0);
static readonly Rational naN = new Rational(0, 0, true);
static readonly Rational positiveInfinity = new Rational(1, 0, true);
static readonly Rational negativeInfinity = new Rational(-1, 0, true);
#endregion
#region Instance fields
readonly BigInteger numerator, denominator;
readonly bool explicitConstructorCalled, isDefinitelyIrreducible;
#endregion
#region Constructors
[DebuggerStepThrough]
public Rational(BigInteger numerator)
: this(numerator, 1, true) { }
[DebuggerStepThrough]
public Rational(BigInteger numerator, BigInteger denominator)
: this(numerator, denominator, false) { }
[DebuggerStepThrough]
private
I'd appreciate comments of anyone who has the time to review the code. The posted code is free to use, so you can make good use of it should you want/need to.
The implementation is basically finished except the
Parse and TryParse methods which I'm still working on.I'm particularly interested in the
FromDouble and TryFromDouble methods which I think are working relatively well but any suggestions, improvements or discovery of any, so far, undetected bugs are very welcomed.```
using System;
using System.Diagnostics;
using System.Globalization;
using System.Numerics;
namespace MathSuite.Core.Numeric
{
[Serializable]
public struct Rational : IFormattable, IEquatable, IComparable, IComparable
{
#region Static fields
const string positiveInfinityLiteral = "Infinity";
const string negativeInfinityLiteral = "-Infinity";
const string naNLiteral = "NaN";
const int fromDoubleMaxIterations = 25;
static readonly Rational one = new Rational(1);
static readonly Rational zero = new Rational(0);
static readonly Rational naN = new Rational(0, 0, true);
static readonly Rational positiveInfinity = new Rational(1, 0, true);
static readonly Rational negativeInfinity = new Rational(-1, 0, true);
#endregion
#region Instance fields
readonly BigInteger numerator, denominator;
readonly bool explicitConstructorCalled, isDefinitelyIrreducible;
#endregion
#region Constructors
[DebuggerStepThrough]
public Rational(BigInteger numerator)
: this(numerator, 1, true) { }
[DebuggerStepThrough]
public Rational(BigInteger numerator, BigInteger denominator)
: this(numerator, denominator, false) { }
[DebuggerStepThrough]
private
Solution
I noticed this because
These are all implicitly
Never mind. After much digging through the code, I see that you are exposing them to the outside world through getters...
This is overkill in my opinion. The likelihood of ever needing to change the implementation is next to nil so having properties for these doesn't make much sense. It just clutters and confuses the code. Minimally, the public getters need to be declared much much closer to the private fields.
naN looked really weird to my eyes. It's almost always seen as NaN. So I looked around a bit and found this.static readonly Rational one = new Rational(1);
static readonly Rational zero = new Rational(0);
static readonly Rational naN = new Rational(0, 0, true);
static readonly Rational positiveInfinity = new Rational(1, 0, true);
static readonly Rational negativeInfinity = new Rational(-1, 0, true);These are all implicitly
private, which explains the camelCasing, but should they be? These are the kinds of static members that usually get exposed to the outside world, particularly when creating structs. Client code is going to need these.Never mind. After much digging through the code, I see that you are exposing them to the outside world through getters...
public static Rational PositiveInfinity { get { return positiveInfinity; } }
public static Rational NaN { get { return naN; } }
public static Rational NegativeInfinity { get { return negativeInfinity; } }
public static Rational Zero { get { return zero; } }This is overkill in my opinion. The likelihood of ever needing to change the implementation is next to nil so having properties for these doesn't make much sense. It just clutters and confuses the code. Minimally, the public getters need to be declared much much closer to the private fields.
Code Snippets
static readonly Rational one = new Rational(1);
static readonly Rational zero = new Rational(0);
static readonly Rational naN = new Rational(0, 0, true);
static readonly Rational positiveInfinity = new Rational(1, 0, true);
static readonly Rational negativeInfinity = new Rational(-1, 0, true);public static Rational PositiveInfinity { get { return positiveInfinity; } }
public static Rational NaN { get { return naN; } }
public static Rational NegativeInfinity { get { return negativeInfinity; } }
public static Rational Zero { get { return zero; } }Context
StackExchange Code Review Q#95681, answer score: 5
Revisions (0)
No revisions yet.