The comment you received makes perfectly sense.
You should try to correct your constructor this way:
public Rational(int numerator, int denominator)
{
Numerator = numerator;
Denominator = denominator;
}
But you actually should rethink the way you handle an attempt to set the denominator to zero. Here throwing a
DivideByZeroException would be a better idea, as it will give an information about an invalid operation attempt, rather than simply swallowing the issue by assigning the denominator a value of one, and thus returning an incorrect result that will be used by the calling code.
You should also take time to study the basis of C# classes and OOP in general, and understand the notions of classes and instances, for example.
Once you got this basic knowledge, you will be able to dive into more complex rational numbers handling, and implement concepts such as LCM and GCD, for example, which will come handy for this kind of valuetype.
On a final note, a rational number is a number, and thus conveys all concepts of a valuetype (immutability, etc...). So I would go here for a
struct instead of a class. This would involve getting rid of your property setters, only allowing the manipulatio of a rational number either by its constructor, or by using basic mathematic functions between rational numbers. But, again, this may be a little too soon for you to consider.
Hope this helps. Kindly.
Submit an article or tip