1

The following code demonstrate simple operator overloading:

Class Position has 3 int field and a method to print them.

class Position
{

    private:
        int x,y,z;

    public:
        void print()
        {
            cout << x << "," << y << "," << z << endl;
        }

        Position (int a, int b, int c)
        : x(4),
        y(50),
        z(23)
        {
            x=a;
            y=b;
            z=c;
        }

        Position operator+(const Position &other);
        Position operator*=(const int &multi);
};

Operators + and *= are overloaded as such:

Position Position::operator+ (const Position &other)
{
    x = x + other.x;
    y = y + other.y;
    z = z + other.z;

    return *this;
}


Position Position::operator*= (const int &multi)
{
    x = x * multi;
    y = y * multi;
    z = z * multi;

    return *this;    
}

The code runs:

int main()
{
    Position p5( 1, 2, 3 );
    p5.print();

    Position p6( 4, 5, 6 );
    p6.print();

    Position p7 = p5 + p6;
    p7.print();

    p7 *= 2;
    p7.print();

    (p7 *= 2) *= 3;
    p7.print();
}

The result yielded are:

1,2,3
4,5,6
5,7,9
10,14,18
20,28,36

Question is why won't the last result perform as it should when it is done in nested?

booyah
  • 45
  • 1
  • 5
  • 9
    Because you return by value, not by reference. – Oliver Charlesworth May 31 '14 at 17:46
  • What is the output if the last p7.print() line, you didn't include it. – CDahn May 31 '14 at 17:48
  • 1
    Because you do not return `Position` by reference from `operator *=`. – blackbird May 31 '14 at 17:49
  • 1
    Your binary `operator+` modifies the LHS operand. This is unlikely to be the desired behaviour. – juanchopanza May 31 '14 at 17:49
  • 1
    See the Binary Arithmetic Operators section of this answer: [Common operators to overload](http://stackoverflow.com/a/4421719/445976) – Blastfurnace May 31 '14 at 17:52
  • 2
    The correct answer has already been given, but your constructor is also strange. It first initialises your members to some hard-coded values, then assigns the real ones. Why doesn't it initialise them to the real ones? – Christian Hackl May 31 '14 at 17:57
  • The answer is correct, but you really shouldn't do this. For the built in `*=` operator this is Undefined Behaviour (no sequence point). In your case the function call introduces a sequence point so it works OK, but it will mislead the reader and it's not best practice. – david.pfx Jun 01 '14 at 02:51
  • @Christian Hackl thanks so much for your guidance and help. – booyah Jun 01 '14 at 05:57

1 Answers1

9
(p7 *= 2) *= 3;

won't work since you're assigning to a temporary, as you return by value in the operator*= function.

For the sub-expression

p7 *= 2

your operator function is called only for its side-effect, as the temporary it returns would be thrown away. To know more about temporaries, read What are C++ temporaries? and Temporary objects - when are they created, how do you recognise them in code?

When you want expression chaining, you've to return by reference like so

Position& Position::operator*= (int multi)
{
    x = x * multi;
    y = y * multi;
    z = z * multi;

    return *this;    
}

Aside: You don't have to pass built-in types by const reference(const int &multi), non-const copy (int multi) should do fine.

legends2k
  • 31,634
  • 25
  • 118
  • 222