-2

I have to implement Rational class in order to get Rational fractions. The header.h file is provided by my instructor so I have to follow up. I also have to write copy constructor in Rational::Rational(const Rational& cRational) function so the object can be copied. I have written my code but the addition of fractions in the output is wrong. Can anybody help me figure this one out? What is wrong with my coding specially in Rational::addition(const Rational &a) or how could i fix it ?

output :

 Begin Rational Class Tests

Testing default constructor: 1/1

Testing std constructor: 1/2

Testing copy constructor: 1/2

Testing addition: 4/4 + 1/2 = 4/4    // it should be 1/2 + 1/2 = 4/4

main function:

int main()
{
    cout << "Begin Rational Class Tests\n\n";

    cout<<"Testing default constructor: ";
    Rational n1;
    n1.printRational();
    cout << endl << endl;

    cout<<"Testing std constructor: ";
    Rational n2(1,2);
    n2.printRational();
    cout << endl << endl;

    cout<<"Testing copy constructor: ";
    Rational n3(n2);
    n3.printRational();
    cout << endl << endl;

    cout<<"Testing addition: ";
    n1 = n2.addition(n3);
    n2.printRational();
    cout <<" + ";
    n3.printRational();
    cout <<" = ";
    n1.printRational();
    cout << endl << endl;
}

Header File:

class Rational {
  public:
    Rational();  // default constructor
    Rational(int, int); //std (initialisation) constructor
    Rational(const Rational&); //copy constructor
    Rational addition(const Rational &);
    void printRational();
  private:
    int numerator;
    int denominator;
};

my program:

//default constructor
Rational::Rational() 
{
   numerator = 1;
   denominator = 1;
}
//initialize constructor
Rational::Rational(int n, int d)
{
     numerator = n;
     if (d==0) 
     {
        cout << "ERROR: ATTEMPTING TO DIVIDE BY ZERO" << endl;
        exit(0); // will terminate the program if division by 0 is attempted
     }
     else
        denominator = d;
}
//copy constructor
Rational::Rational(const Rational& cRational)
{
    numerator = cRational.numerator;
    denominator = cRational.denominator;
}
//addition
Rational Rational::addition(const Rational &a)
{
     numerator = numerator * a.denominator + a.numerator * denominator;
     denominator = denominator * a.denominator;
     return Rational(numerator,denominator);
}

void Rational::printRational()
{
    cout << numerator << "/" << denominator ;
}
muzzi
  • 382
  • 3
  • 10
  • I expect you are getting a compiler warning for your `addition`. Are you? If so, it should point you in the right direction. –  May 15 '18 at 04:45
  • 1
    `In member function ‘Rational Rational::addition(const Rational&)’:` `code.tio.cpp:45:1: warning: no return statement in function returning non-void` – melpomene May 15 '18 at 04:48
  • 1
    I question the need for a copy constructor. [The Rule of Zero applies to this class.](http://en.cppreference.com/w/cpp/language/rule_of_three) – user4581301 May 15 '18 at 04:50
  • I have no idea why my instructor forced us to write code in copy constructor. – muzzi May 15 '18 at 04:51
  • Another hint: The declaration should be `Rational addition(const Rational &) const;`. – melpomene May 15 '18 at 04:53
  • I have just added return statement in addion(const Rational&) but the output is still wrong please have look in the output. It gives 4/4 but it should be 1/2. – muzzi May 15 '18 at 04:54
  • 1
    Why would you expect that? 1/2 + 1/2 is 1 4/4 is 1. I see no problem other than `addition` destroys `this` in the process. – user4581301 May 15 '18 at 05:04
  • 3
    Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See [mcve]. – Ulrich Eckhardt May 15 '18 at 05:10
  • 2
    Other things to ask your instructor: why did they name it `Rational::addition` and not `Rational::operator +=`, also `Rational::printRational` not `std::ostream & operator<<` – Caleth May 15 '18 at 06:44
  • Please [edit] your code to reduce it to a [mcve] of your problem. Your current code includes much that is peripheral to your problem - a minimal sample normally looks similar to a good unit test: only performing one task, with input values specified for reproducibility. – Toby Speight May 15 '18 at 09:59

2 Answers2

1

Just create new variables with different names for new values of numerator and denominator in addition(), somewhat like this...

int n, d;  //you could use better names
n = numerator * a.denominator + a.numerator * denominator;
d = denominator * a.denominator;
return Rational(n, d);

I checked, this works here.

In general, don't use same names for two variables in the same scope.

Gaurav Singh
  • 456
  • 6
  • 17
1

Your addition function is modifying the member variables of *this, which causes strange things to happen.
A more appropriate prototype would be

Rational addition(const Rational &) const;

as that would let the compiler tell you that you were doing something weird.

You can either use local variables instead of assigning to the members, or you can do without intermediate variables completely:

Rational Rational::addition(const Rational &a)
{
     return Rational(numerator * a.denominator + a.numerator * denominator, 
                     denominator * a.denominator);
}
molbdnilo
  • 64,751
  • 3
  • 43
  • 82