0
votes

This is a homework, and we don't learn the structs in C# (and I am learn now). So I go to the msdn, but I don't find what my program's problem... Yes, and I can't translate fully assigned to my native languange, and I don't understadn what the compiler thinks.

UML of my homework: (we must use it) Interface: IComparable

  • +Numerator{get;}:long
  • +Denominator{get;}:long
  • +Rational(numerator:long, denominator:long)
  • +GCD(a:long, b:long):long
  • +Equals(r:Rationaol):bool
  • ...

And we mustn't implement another methods or params.

I write this homework in Java with classes and that work correctly.

I have a big problem, I don't understand what mean the following erros:

Error 2 Backing field for automatically implemented property 'Rational.Rational.Denominator' must be fully assigned before control is returned to the caller. Consider calling the default constructor from a constructor initializer.

Error 3 Backing field for automatically implemented property 'Rational.Rational.Numerator' must be fully assigned before control is returned to the caller. Consider calling the default constructor from a constructor initializer.

Error 4 The 'this' object cannot be used before all of its fields are assigned to

my code: namespace Rational

{
    //       (Rational is underline)     here the 2 and 3 error
    public struct Rational : IComparable<Rational>
    {
        public long Numerator { get; set; }
        public long Denominator { get; set; }

        public Rational(long num, long den)
        {
             // and here (GCD is underline) the 4. error
            long simple = GCD(num, den);
            this.Numerator = num/simple;
            this.Denominator = den/simple;
        }

        public long GCD(long a, long b)
        {
            long c = Math.Abs(a);
            long d = Math.Abs(b);
            if (d == 0) return c;
            return GCD(d, c % d);
        }

        public override string ToString()
        {
            if (Denominator==1)
            {
                return Numerator + "";
            }
            return Numerator+"/"+Denominator;
        }

        public override bool Equals(object obj)
        {
            bool result = false;
            if (obj is Rational)
            {
                Rational r = (Rational)obj;
                result = this.Equals(r);
            }
            return result;            
        }

        public bool Equals(Rational r)
        {
            if ((object)r ==null)
            {
                return false;
            }
            return this.Denominator == r.Denominator && this.Numerator == r.Numerator;
        }

        public int CompareTo(Rational other)
        {
      ...
4

4 Answers

1
votes

You can either add a this() to your constructor, or replace the auto-properties by properties backed by a field.

 public Rational(long num, long den)
 {
         // and here (GCD is underline) the 4. error
        long simple = GCD(num, den);
        this.Numerator = num;
        this.Denominator = den;
 }

Here you access the instance method GCD before you assigned a value to the auto generated fields backing your properties.

You should make this method static.

Next you will get the same error again, this time because you access the auto property Numerator. You can fix that by keeping the auto properties and adding :this() to the constructor:

 public Rational(long num, long den)
   :this()
 {

That results in the fields being initialized to 0 before your own constructor code runs.

The alternative is switching to fields:

public struct Rational : IComparable<Rational>
{
    private long _numerator;
    private long _denominator;

    public long Numerator { get{return _numerator;}; set{_numerator=value;} }
    public long Denominator{ get{return denominator;}; set{_denominator=value;} }

    public Rational(long num, long den)
    {
         // and here (GCD is underline) the 4. error
        long simple = GCD(num, den);
        this._numerator = num;
        this._denominator = den;
    }

Apart from this your code has few more issues:

1) You're using a mutable struct. That's usually bad design. Remove the setter from your properties or make it private.

2) You don't override GetHashCode() to be consistent with Equals (or it's just not shown in your code excerpt)

3) I recommend implementing IEquatable<Rational>. You already implemented Equals(Rational), so you don't need to add any additional methods.

4) Your code produces int overflows very easily. Consider using BigIntegers instead of longs.

5) Unless you normalize your rational (denominator >0 and dividing both by the GCD) you get mathematically equivalent rationals that don't compare as equal.

1
votes

You need to call the default constructor for this to work:

public Rational(long num, long den) : this()
{
    // and here (GCD is underline) the 4. error
    long simple = GCD(num, den);
    this.Numerator = num;
    this.Denominator = den;
}

A bigger problem here is that you have a mutable struct. This is never a good idea. I would make it:

public long Numerator {get; private set;}
0
votes

I notice in your constructor for Rational that you are calling GCD and storing the result in simple, but you don't use the result.

0
votes

Make your GCD function static.

It doesn't use any instance members, and because it's called before instance members are set, it can't.