CODEWITHSUNDEEP
c++
Lazer
Lazer

Reputation: 94930

implementing prefix operator++

From http://www.learncpp.com/cpp-tutorial/97-overloading-the-increment-and-decrement-operators/

Class declaration

class Digit
{
private:
    int m_nDigit;
public:
    Digit(int nDigit=0)
    {
        m_nDigit = nDigit;
    }

    Digit& operator++();
    Digit& operator--();

    int GetDigit() const { return m_nDigit; }
};

Their implementation of operator++

Digit& Digit::operator++()
{
    // If our number is already at 9, wrap around to 0
    if (m_nDigit == 9)
        m_nDigit = 0;
    // otherwise just increment to next number
    else
        ++m_nDigit;

    return *this;
}

My alternate implementation of operator++

Digit& Digit::operator++()
{
    return Digit(m_nDigit == 9 ? 0 : (m_nDigit + 1));
}

I wanted to know

  1. if there are any downsides of creating a new object like I have done, and
  2. about how to choose one of these implementations?

Upvotes: 0

Views: 230

Answers (4)

Topo
Topo

Reputation: 5002

You have a problem with the ternary operator. Also you are returning a copy of the object not the object itself, and if you are trying to overload the operator to work like a built-in type, you should always return a reference to the object.

I think you should do it like this:

Digit& Digit::operator++()
{
    m_nDigit = (m_nDigit == 9 ? 0 : m_nDigit++);
    return *this;
}

Or like this:

Digit& Digit::operator++()
{
    m_nDigit = (++m_nDigit % 10);
    return *this;
}

The downside of creating a new object is that in code like:

Digit d;
++d;

I would expect the value of d to change and with a new object it doesn't. This operator is in many cases used like this (without assigning it to a variable) so if you don't increase and return the same object, you can't use it like that.

Upvotes: 4

littleadv
littleadv

Reputation: 20282

In your alternative implementation you have 2 issues:

  1. instead of m_nDigit = 9 do m_nDigit == 9. Currently m_nDigit will always be 9, and the return value will always be 0.

  2. you're supposed to change the value of m_nDigit. When returning 0 - you don't.

The return statement is problematic because the operator is expected to change the value of the operand, not to create a new object.

edit

To clarify the problem, consider code:

Digit x;
x++;

What would you expect x to be as the result of this code? I would expect it to be 1. Using your operator, it remains unchanged.

Upvotes: 3

user405725
user405725

Reputation:

  1. m_nDigit = 9 is an assignment and will always be evaluated to true.
  2. You return a copy of the object but return type is a reference. This may bite you (see Is returning a temp-object by reference possible).
  3. Why not m_nDigit = (m_nDigit + 1) % 9;?

Upvotes: 2

Mankarse
Mankarse

Reputation: 40633

There certainly are downsides to creating an object as you have done:

  1. It will not compile. It is impossible to bind a non-const reference to a temporary.

  2. Even if it did compile it would not perform the correct operation (your version makes operator++ set m_nDigit to 9, the other version increments m_nDigit, wrapping around when 10 is reached).

  3. Even if it did compile and do the right thing, it would be very unsafe. The returned reference would be bound to a temporary object which would be destroyed before the function returned, this would mean that any attempt to access the object referenced by the return value of operator++ would result in undefined behaviour.

To choose between the two implementations, you should pick the one which compiles, is correct and is safe.

Upvotes: 4

Related Questions