Making sure I understand why I had an error (C++)

[sweenish]

I have a Money class, with overloaded multiplication and negative sign operator overloads.

In a different class, I was attempting to store the negative of a product, but I was getting an error about passing 'this' argument. Relevant code and errors follow.

I just want to make sure I understand the issue. It's because my return types are const, so the anonymous const Money object that I return cannot be manipulated by the negative sign.

If I defined a move constructor, and simply returned by value, it would work as I expected it to.

Please let me know how right or wrong that assessment is, and if it's wrong, but the real issue is. I can also upload more code if need be. I don't think I'm completely right, because if it was a const issue, the compiler usually does a better job of telling me so. I'm using g++ 7.3.

Money negative sign overload:

const Money Money::operator -()
{
        dollars = -dollars;
        cents = -cents;

        return Money(dollars, cents);
}

Relevant Money multiplication overload (These are friend functions):

const Money operator *(const Money &left, const int right)
{
    int leftPennies = left.makePennies();
    int product = leftPennies * right;

    return Money(product / 100, product % 100);
}

const Money operator *(const int left, const Money &right)
{
    return (right * left);
}

The usage that threw an error:

// cash is a private data member of my other class
// This is in one of my constructor bodies
cash = -(hotdogSupply * wholesaleHotdogCost);

The error:

HotdogStand.cpp: In constructor ‘MyAwesomeBusiness::HotdogStand::HotdogStand()’:
HotdogStand.cpp:18:46: error: passing ‘const MyAwesomeBusiness::Money’ as ‘this’ argument discards qualifiers [-fpermissive]
   cash = -(hotdogSupply * wholesaleHotdogCost);
                                              ^
In file included from HotdogStand.hpp:4:0,
                 from HotdogStand.cpp:1:
Money.hpp:22:15: note:   in call to ‘const MyAwesomeBusiness::Money MyAwesomeBusiness::Money::operator-()’
   const Money operator -();

 

[Merad]

Yes, I think you're correct. I'm also fairly sure the unary minus operator is intended to be const, so the signature should be:

const Money Money:perator -() const;

After all, there's no reason to mutate 'this' when you're returning a copy.

 

[sweenish]

Good tip! I will make that change.

EDIT: Just did a little looking on what's considered best for unary minus between returning a copy and returning a reference, and found out about reference qualifiers. Pretty interesting. I'll definitely be utilizing that in a later project. This one is fine with the single return by value version.

https://stackoverflow.com/questions...e-unary-minus-operator-of-an-rvalue-reference

 

[mv2devnull]

The problem was not in what you did return, but what you did do, like Merad implied:

Money::operator -()
{
       dollars = -dollars; // trouble
       cents = -cents;

In use:

const Money foo;
auto bar = -foo; // error, cannot modify foo

Trivial implementation:

Money::operator- () const
{
  return Money( -dollars, -cents );
}

The return type of the function can be

Money
const Money
const Money &

but not non-const reference, for we return a temporary unnamed automatic variable.

 

[sweenish]

Yeah, I did switch my function body to create a new object to return.

I was thinking that even if my unary minus had been correctly implemented from the get-go, then I would have still got the error when trying to take the negative of an expression.

But I put together a little test and it does indeed work the way I would have expected. Good to know. I can usually decrypt the compiler error messages well enough, but this one was throwing me.

 

[mv2devnull]

Probably due to the unnamed temporary in:

cash = -(hotdogSupply * wholesaleHotdogCost);

It is similar as in:

Money cash;
Money cost;
int supply;

auto temp = supply * cost;
cost = -temp; // error

It does not matter whether cost and supply are const or not; the operator* accepts all and spits out const Money. The temp is thus const, and there is an attempt to call the non-const operator- on it.

In your version there is no separate variable 'temp'; only an unnamed temporary Money object. GCC had no words for that.


You were actually lucky: if your operator* would return a non-const Money, then your operator- would have been "ok" and you had had a silent logical error.

 

[sweenish]

The temp is thus const, and there is an attempt to call the non-const operator- on it.

The second part of that sentence is the key I was missing. I thought I had all my types and functions set up properly; obviously I did not.

This was an enlightening discussion. Thank you all very much!