Can't get calculations right

cdiaz851 (2)
I'm writing code to make a basic Rational class but i keep hitting a dead
end.
My r1.add(r2) keeps resulting in -426974861 / 44086 (should be 5/2)
My r1.multiply(r2) keeps resulting in 1922985196 / 44086
My equals always results in 0 even when i change the values to result in1
My r2.to_string results in nothing
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
//Rational.h 
    #include <string> //for string member

    using namespace std;

    class Rational{
    private:
        int p;
        int q;
    public:
        Rational();
        Rational(int numerator, int denominator);
        Rational(int numerator);
        void set(int numerator, int denominator);
        int getNumerator() const;
        int getDenominator() const;
        double to_double() const;
        Rational add(Rational rhs) const; // rhs = second rational
        Rational multiply(Rational rhs) const;
        bool equals(Rational rhs) const;
        string to_string() const;
        void input();
    };


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
 
    // Rational.cpp
    #include "Rational.h"
    #include <iostream>
    #include <cstdlib>
    #include <string>
    #include <iomanip>

    Rational::Rational(int numerator, int denominator)
    {
        set(numerator, denominator);
    }

    Rational::Rational(int numerator)
    {
        Rational(numerator, 1);
    }

    Rational::Rational()
    {
        Rational(0,1);
    }

    void Rational::set(int numerator, int denominator)
    {
        p = numerator;
        q = denominator;
        if(q == 0){
            cerr << "Denominator cannot be 0";
            exit(EXIT_FAILURE);
        }
    }

    int Rational::getNumerator() const
    {
        return p;
    }

    int Rational::getDenominator() const
    {
        return q;
    }

    double Rational::to_double() const
    {
        return static_cast<double>(p)/q;
    }

    Rational Rational::add(Rational rhs) const
    {
        return Rational(p*rhs.q + q*rhs.p, q*rhs.q);
    }

    Rational Rational::multiply(Rational rhs) const
    {
        return Rational(p*rhs.p, q*rhs.q);
    }

    bool Rational::equals(Rational rhs) const
    {
        return (p*rhs.q == q*rhs.p);
    }

    string Rational::to_string() const
    {
        return p + " / " + q;
    }

    void Rational::input()
    {
       int numerator, denominator;
       cout << "Enter numerator and denominator?";
       cin >> numerator >> denominator;
       if (denominator == 0){
           cerr << "Denominator cannot be 0";
           exit(EXIT_FAILURE);
       }
       set(numerator, denominator);
       cout << "Rational: " << numerator << " / " << denominator;
    }


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
    // main.cpp

    #include <iostream>
    #include "Rational.h"
    #include <string>
    int main()
    {
        Rational r1;
        Rational r2(1, 2);
        Rational r3(2);
        double d = r2.to_double();
        cout << "d = " << d << endl;
        Rational sum = r1.add(r2);
        cout << "r1 + r2 = " << sum.getNumerator() << " / " << 
        sum.getDenominator() << endl;
        Rational product = r1.multiply(r2);
        cout << "r1*r2 = " << product.getNumerator() << " / " << 
        product.getDenominator() << endl;
        bool same = r1.equals(r2);
        cout << "r1 = r2 ?" << same << endl;
        cout << "r2 = " << r2.to_string() << endl;
        r1.input();
    }



I expect r1.add(r2) to result in 1 / 2
I expect r1.multiply(r2) to result in 0 / 2
And i expect r2.to_string to result in "1 / 2"
What is causing my functions to return these random numbers?
Last edited on
jonnin (11438)
welcome! While I look, please edit your post and click the <> tags on the mini editor bar around your code ... it will be easier to read.
jonnin (11438)
oh. r1 is uninitialized at the point where you add it and display the results. Its just garbage + garbage = garbage.
maybe you needed a call to input?

also, you can create operator + and such for this class, so you can say r1+r2 instead of the "method add" syntax. Once you get it working, we can show you this if you want to see it, or look at an example of operator overload on google.
Last edited on
cdiaz851 (2)
Isn't r1 initialized in main.cpp? I wouldn't be able to change the syntax of the functions (part of the instructions). "Rational r1" is supposed to go to "Rational()" in "Rational.h", making r1 have p=0 and q=1. Is there something i'm doing wrong?
Last edited on
dutch (2548)
Yes, r1 would be initialized by the default ctor. But it isn't written correctly. It should be:

1
2
3
4
5
6
7
8
9
 
Rational::Rational(int numerator)
    : Rational(numerator, 1)
{
}

Rational::Rational()
    : Rational(0, 1)
{
}

If you could change the header you could just have one ctor prototyped like this in the class:

 
 
Rational(int numerator=0, int denominator=1);

Last edited on
Niccolo (720)
cdiaz851

I would add to dutch's response that this:

1
2
3
4
 
  Rational::Rational()
    {
        Rational(0,1);
    }


Is not doing what you expect. What does happen you can see if you trace through that in a debugger.

Effectively, this creates a Rational inside the default constructor of a Rational, but it's temporary, and then evaporates when the default constructor finishes.

It doesn't set p and q of "this" Rational, it sets the p and q of the temporary, but that disappears.

dutch's code is the way of calling another constructor as a function (what you're calling as a function in your version is actually creating another temporary Rational), but it was added to the C++ language at some point (I don't remember when), and wasn't always legal code either (it's valid in modern C++).

Instead, if you can't modify the code per dutch, then call set in the default constructor.

Then, read your other constructor taking an int for the exact same problem, as that is doing this, too (creating a temporary Rational that disappears, leaving "this" Rational uninitialized).


Last edited on
dhayden (5798)
If you can't change the header, you could just have all constructors call set(). It's a little less efficient, but it might be easier to understand if you haven't studied base class constructors yet.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
    Rational::Rational(int numerator, int denominator)
    {
        set(numerator, denominator);
    }

    Rational::Rational(int numerator)
    {
        set(numerator, 1);
    }

    Rational::Rational()
    {
        set(0,1);
    }
Topic archived. No new replies allowed.