C++ Operator - method

Question

Here is my code:

struct Goods {
    int amount;
    int rarity;

    Goods operator -(const goods &other);
};

and this is how I attempt to implement operator- method:

Goods Goods::operator -(const Goods &other) {
    this->amount = amount - other.amount;
    this->rarity = rarity - other.rarity;
}

But this code doesn't work as intended. The actual value of this test

TEST(Mytest, GoodsMinus) {
    Goods g1{3, 4};
    Goods g2{2, 1};
    ASSERT_EQ(1, (g1 - g2).amount);
    ASSERT_EQ(3, (g1 - g2).rarity);
}

The actual value of first assert is 9435404, which is obviously wrong. What am I doing wrong here?

Does this answer your question? [What are the basic rules and idioms for operator overloading?](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading) — Evg, Jan 18 '20 at 18:19

score 3 · Answer 1 · answered Jan 18 '20 at 18:21

Your method isn't returning anything, and to add insult to injury, it's operating as a -=-like method, modifying the left-hand operand instead of returning a new Good while leaving the inputs untouched (if implemented as a method on the instance, which isn't the usual approach, it should be a const method).

I'm going to strongly recommend you read What are the basic rules and idioms for operator overloading?, because even once you fix these issues, you're heading down the wrong path for writing maintainable, idiomatic code.

Liuka · Answer 2 · 2020-01-18T18:32:35.033

Your operator - should return a new object which stores the new values calculated, in your case, as the difference of each member. In the code you provided you're (semantically) actually implementing the operator -=, in which, instead, you modify directly the data of the current object; you're in fact accessing this->... data and not a new object.

Therefore your - operator should look like:

Goods Goods::operator-(const Goods &other) const {
    Goods r;
    r.amount = amount - other.amount;
    r.rarity = rarity - other.rarity;

    return r;
}

EDIT: As sugested I added const to the operator which means you can use it even with const object (which makes sense as you're not modifing the object internal state)

C++ Operator - method

2 Answers2