Chaining methods is giving me unexpected results, and the arguments are being evaluated in reverse order

Cliffnotes:

I've gotten method chaining to work as I expected in one case, but in another case, there is something funny going on.

I expect these two example to have the exact same output:

As expected example

Not as expected example

I've done a lot of chaining using Javascript, so when I learned that you can chain class methods in a similar way in C++, I wanted to give it a try. I ran into some problems though. I'm not sure if it's the chaining causing the problems, or something else.

I'm doing the chaining by returning references to this. For example:

Class LLL
{
public:
    LLL & addNode( ... );

and then the add Node method ends with:

    ...
    return *this;
}

There's 2 pertinent classes and 3 pertinent methods.

I have a linear linked list class, LLL and a Node classs, Node. To keep this as simple as possible a node simply holds an int (called guid) and a next and prev pointer. A linear linked list puts a bunch of nodes in a LLL. This all works fine, but when I add nodes using chaining I sometimes get odd results.

An example where things work like I expect:

    // Create 3 node objects & change their guids
Node node1; node1.change(1); // change(n) outputs "-n-"
Node node2; node2.change(2);
Node node3; node3.change(3);

    // Create a linear linked list object
LLL lll;
    // Add the nodes to the LLL and show the LLL
lll.addNode(node1).addNode(node2).addNode(node3).show();

// Expected and real output:
-1--2--3-123 

Try it out with this Codepad

Now I wanted to try it with just one node object:

An example where I don't understand what is going on:

Node nod;
LLL lll;
lll.addNode(nod.change(1)).addNode(nod.change(2)).addNode(nod.change(3)).show();

// Expected output:
-1--2--3-123
// Output:
-3--2--1-111 

// Why are the changes being made in reverse order? And why do all three
//    nodes have a guid of 1?

Try it out with this Codepad

It seems like the two examples above should have identical results.

I think I must be misunderstanding something about what happens to the single node object in the second example.

You can look at the code in either of the codepads above, and I'll include the complete code for the second example below. I set up the code to be one huge file, so that I can put it on codepad, but I'll comment in what would be node.h, node.cpp, lll.h, lll.cpp, and main.cpp:

#include <cstdlib>
#include <iostream>
#includ开发者_StackOverflow社区e <cstring>
#include <ctime>
using namespace std;

// NODE.H ======================================================================*/
// This class holds the data
class Node
{
public:
    Node();
    Node(int guidIn);
    Node(const Node & nodeIn);
    Node & change(int guidIn);
    void commonConstructor();
    // Get the next node
    Node * next();
    // Set the next node
    void   next(Node * nodeIn);
    // Get the previous node
    Node * prev();
    // Set the previous node
    void   prev(Node * nodeIn);
    Node & show();
private:
    int guid;
    Node * nextNode;
    Node * prevNode;
};
/* EOF
   =============================================================================*/

// LLL.H =======================================================================*/
// This is a LLL to hold nodes
class LLL
{
public:
    LLL();
    ~LLL();
    LLL & addNode(const Node & nodeIn);
    LLL & show();
private:
    Node * head;
};
/* EOF
   =============================================================================*/

// NODE.CPP ====================================================================*/
Node::Node()
{
    guid = 0;
    commonConstructor();
}
Node::Node(int guidIn)
{
    guid = guidIn;
    commonConstructor();
}
Node::Node(const Node & nodeIn)
{
    guid = nodeIn.guid;
    commonConstructor();
}
Node & Node::change(int guidIn)
{
    guid = guidIn;
    cout << "-" << guidIn << "-";
    return *this;
}
void Node::commonConstructor()
{
    nextNode = NULL;
    prevNode = NULL;
}
Node * Node::next()
{
    return nextNode;
}
void Node::next(Node * nodeIn)
{
    nextNode = nodeIn;
}
Node * Node::prev()
{
    return prevNode;
}
void Node::prev(Node * nodeIn)
{
    prevNode = nodeIn;
}
Node & Node::show()
{
    cout << guid;
    return *this;
}
/* EOF
   =============================================================================*/

// LLL.CPP =====================================================================*/    
LLL::LLL()
{
    head = NULL;
}
LLL::~LLL()
{
    Node * temp = head;
    while(head)
    {
        temp = head;
        head = head->next();
        delete temp;
    }
}
LLL & LLL::addNode(const Node & nodeIn)
{
    Node * tempNode = new Node(nodeIn);
    if(!head)
    {
        head = tempNode;
    } else
    {
        Node * temp = head;
        while(temp->next())
        {
            temp = temp->next();
        }
        temp->next(tempNode);
    }
    return *this;
}
LLL & LLL::show()
{
    Node * temp = head;
    while(temp)
    {
        temp->show();
        temp = temp->next();
    }
    return *this;
}
/* EOF
   =============================================================================*/

// MAIN.CPP ====================================================================*/    
int main()
{
    Node node;
    LLL lll;
    lll.addNode(node.change(1)).addNode(node.change(2))
        .addNode(node.change(3)).show();

    cout << "\n";
    return 0;
}
/* EOF
   =============================================================================*/

In the second version, you are invoking many operations on the same node, and not considering the implications of evaluation-order. The expression a.f(b) is kind of the same as A::f(a, b), where a is of class A, and the compiler is free to evaluate the parameters a and b in any order it sees fit.

In your case, when the compiler sees the the expression, a.foo(b.bar(1)).foo(b.bar(2)), it is free to evaluate a.foo(b.bar(1)) after it evaluates b.bar(2), and then call foo, which obviously yields different behaviour from what you expect.

In short, never mutate and use an object multiple times in the same expression. My philosophy is, if at all possible, never mutate objects.

C++ does not guarantee what order sub-expressions will be evaluated in.

In the example where you get this: -3--2--1-111 What's happening probably, is this:

lll.addNode(   // fourth
node.change(1) // third
)
.addNode(      // fifth
node.change(2) // second
)
.addNode(      // sixth
node.change(3) // this is first
)
.show();       // last

Since the last value given to node was 1, and you are passing a reference to that node in all to three calls to addnode, all nodes of lll get the same value.

Technically speaking this is undefined behaviour as you are changing one variable (node) multiple times between sequence points. I also guess that you MAY get desired output from the list if you put node to the list by value, not by const reference but it still is undefined behaviour.