Making "cout << obj1 + obj2 << endl;" work

Question

So, I have gotten intra-class operator overloading working for obj1 + obj2:

fraction operator+ (fraction op);

Similarly, cout << obj is working by overloading operator overloading ostream:

ostream& operator<< (ostream& os, fraction& frac);

However, if I try to combine the two, all hell breaks loose.

fraction.cpp:77:27: error: no match for ‘operator<<’ in 
‘std::operator<< [with _Traits = std::char_traits<char>]((* & std::cout), ((const char*)"Sum: ")) 
<< f1.fraction::operator+(f2)’

Here is the code:

#include <iostream>
using namespace std;

class fraction
{
    private:
        int n, d;

    public:
        fraction ()
        {
            this->n = 1;
            this->d = 0;
        }

        fraction (int n, int d)
        {
            this->n = n;
            this->d = d;
        }

        int getNumerator ()
        {
            return n;
        }

        int getDenominator ()
        {
            return d;
        }

        fraction operator+ (fraction op)
        {
            return *(new fraction(this->n*op.d + op.n*this->d, this->d*op.d));
        }

        fraction operator- (fraction op)
        {

            return *(new fraction(this->n*op.d - op.n*this->d, this->d*op.d));
        }

        fraction operator* (fraction op)
        {
            return *(new fraction(this->n*op.n, this->d*op.d));
        }

        fraction operator/ (fraction op)
        {
            return *(new fraction(this->n*op.d, this->d*op.n));
        }
};

ostream& operator<< (ostream& os, fraction& frac)
{
    int n = frac.getNumerator();
    int d = frac.getDenominator();

    if(d == 0 && n == 0)
        os << "NaN";
    else if(d == 0 && n != 0)
        os << "Inf";
    else if(d == 1)
        os << n;
    else
        os << n << "/" << d;
}

int main ()
{
    fraction f1(2, 3);
    fraction f2(1, 3);

    cout << f1 << " " << f2 << endl;

    /*
    cout << "Sum: " << f1+f2 << endl;
    cout << "Difference: " << f1-f2 << endl;
    cout << "Product: " << f1*f2 << endl;
    cout << "Quotient: " << f1/f2 << endl;
    */

    return 0;
}

Help. D:

You are leaking memory like crazy! Replace `return *(new fraction(...));` with `return fraction(...);`. — BoBTFish, Jul 29 '13 at 15:23
Stop using `new`. Possibly get rid of your current learning material in favour of [a decent one](http://stackoverflow.com/q/388242/46642). — R. Martinho Fernandes, Jul 29 '13 at 15:23
Don't do `return *(new T())` if you're trying to get an object, just do `return T()`. — David G, Jul 29 '13 at 15:23
C++ isn't java. As others have pointed out, this is leaking like a boat with a screen-door-hull. — WhozCraig, Jul 29 '13 at 15:25
Some other comments: I'd normally expect `fraction::fraction( int n, int d = 1 )`, to get implicit conversions from `int`. In which case, `operator+` et al. should _not_ be members; the usual convention would be to define `operator+=` et al. as members, and then `operator+` as a free function, using `+=` in its implementation. If you have the implicit conversion, and `operator+` is a member, `fraction( 1, 2 ) + 3` works, but `1 + fraction( 2, 3 )` doesn't. — James Kanze, Jul 29 '13 at 15:33
Thanks guys, just trying to head first into some of C++'s unique features instead of playing it safe with a .display() method. As you can tell, I'm from a C and Java background. Still waiting for my copy of the "C++ Primer" to ship. — peteykun, Jul 29 '13 at 16:26

Luchian Grigore · Accepted Answer · 2013-07-29T15:25:45.807

9

The immediate problem is that

ostream& operator<< (ostream& os, fraction& frac)

won't accept a temporary because frac isn't a const reference - change it to

ostream& operator<< (ostream& os, const fraction& frac)

operator+ between two fractions will return a temporary which can't bind to a non-const reference.

There's also a very serious memory leak in these cases:

fraction operator+ (fraction )
{
   return *(new fraction(this->n*op.d + op.n*this->d, this->d*op.d)); 
}

new will return a dynamically allocated object which you then copy and return from the function.

Just do it like most people:

fraction operator+ (const fraction& op) const
{
   return fraction(this->n*op.d + op.n*this->d, this->d*op.d); 
}

(note the two extra consts and the pass by reference)

edited Jul 29 '13 at 15:25

answered Jul 29 '13 at 15:22

Luchian Grigore

253,575
64
457
625

3

That's the immediate problem, but you should probably also point out that `operator+` et al. should be const, and that the usual convention would have all of the `fraction` arguements passed by reference to const (even though it probably doesn't make a difference with this class). – James Kanze Jul 29 '13 at 15:24
You should probably mention how his stream operator isn't returning anything. – Rapptz Jul 29 '13 at 15:27
3

As for "Just do like most people": I don't know of anyone who makes `operator+` a member. Most of the time, you just inherit from something like `ArithmeticOperators`, and get it for free. If not, it's usually a free function using `+=` in its implementation. (But of course, your version is perfectly fine too, and for a class this simple, maybe preferable.) – James Kanze Jul 29 '13 at 15:27
I make it a member as I didn't know any different. I've never used a language that gives you so many ways to do the same thing and most of them are wrong most of the time. – Neil Kirk Jul 29 '13 at 15:31
@NeilKirk No truer words have been spoken than the last sentence in your prior comment, save to say that by-definition, something that is "wrong" is *not* a way to do something. *C++ isn't easy*. But you have an *enormous* wealth of people here that will help you if you truly put effort into it on your side. The payoff is worth it, so stick to it. – WhozCraig Jul 29 '13 at 15:35
@NeilKirk In perl, there is more than one way to do it. In C++, determine the number of ways to do something is undefined behavior. – Yakk - Adam Nevraumont Jul 29 '13 at 15:36

Making "cout << obj1 + obj2 << endl;" work

1 Answers1