1

Please note, that this question relates to an assignment for school.

We are building a custom Fraction class with most operators being overloaded. Most of them are not giving me problems. However, this part of the driver is not working for me:

cout << "testing post++ with f2 = f1++;" << '\n';
f2 = f1++;
cout << "f1 : " << f1 << '\n';
cout << "f2 : " << f2 << '\n';
assert(f1 == Fraction(6));
assert(f2 == Fraction(5));
cout << "ok\n\n";

What happens is that f2 gets assigned the value of f1++ and not pre-incremented f1, which is what the assert assumes should be there.

My operator looks like:

Fraction Fraction::operator++(int a)
{
    numer = (numer + denom);
    normalize();
    return *this;
}

Now, I'm scratching my head over this because in my head the logic is that ++ has precedence over the assignment operator, so I'd have expected the asserts to test f1 and f2 with the same values for a post++ operation. For the ++pre overload, the assert values are equal to each other in the driver.

My question is, why should f2 take the pre-incremented value of f1, and how could I modify my operators to achieve this, or could this be an error from the prof?

Assignment operator:

Fraction& Fraction::operator=(const Fraction &rhs) {
    numer = rhs.getNumer();
    denom = rhs.getDenom();
    return *this;
}
Sam
  • 7,252
  • 16
  • 46
  • 65
SpaceSteak
  • 224
  • 2
  • 15
  • 2
    [related FAQ](http://stackoverflow.com/questions/4421706/), see "unary arithmetic operators" heading – fredoverflow Jul 05 '14 at 10:46
  • 3
    Hint: `a++` should return the value of `a` ***before*** you modify it. Thus `return *this;` is a no-no for the implementation. – Daniel Frey Jul 05 '14 at 10:47
  • btw, in addition to my answer, why in the world do you actually take the trouble and post a *screenshot* of the output rather than just copying the text from the command-line window? – Christian Hackl Jul 05 '14 at 11:01
  • T operator ++(int) { T tmp(*this); ++(*this); return tmp; } –  Jul 05 '14 at 11:04
  • @ChristianHackl because ... I like using my macro that uploads a portion of screen to imgur? :p – SpaceSteak Jul 05 '14 at 11:10

2 Answers2

2

Post-increment returns a value before the increment. That is why when you see return *this in a postfix ++ or -- operator you should immediately know that the implementation is wrong.

The correct sequence of actions is as follows:

  • Make a copy of *this
  • Do the increment
  • Return the copy.

Assuming that your Fraction class has a properly functioning copy constructor, the fix is very easy:

Fraction Fraction::operator++(int a)
{
    Fraction res(*this);
    // The following two lines are most likely shared with the prefix ++
    // A common trick is to call ++*this here, to avoid code duplication.
    numer = (numer + denom);
    normalize();
    return res;
}
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
2

When you have problems with overloaded operators as in your example, it's often helpful to look behind the syntactic sugar and see what the calls "really" look like.

f2 = f1++;

This one actually translates as:

f2.operator=(f1.operator++(0));

It's even clearer if you assume for one moment that these are really just ordinarily named functions:

f2.Assign(f1.PostIncrement());

Now it should be obvious what happens. Your PostIncrement function is called first, and its result is passed as an argument to Assign.

There is nothing magic about overloaded operators. They are just functions with special names. There are no special rules for returning values or passing arguments. Thus,

Fraction Fraction::operator++(int a)
{
    numer = (numer + denom);
    normalize();
    return *this;
}

, imagined like this:

Fraction Fraction::PostIncrement()
{
    numer = (numer + denom);
    normalize();
    return *this;
}

does exactly what you wrote: It increments itself, then returns itself.

If you want the same post-increment semantics as the built-in types, then you have to implement those semantics manually. In your operator++(int), create a temporary copy of *this first, then increment, then return the temporary copy.

Christian Hackl
  • 27,051
  • 3
  • 32
  • 62