1

I've written a member function of class Node to read a tree of Nodes in postfix order.

It will be called by the Node instance which is the root node of the tree.

So: N.postfix();

these appear to be illeagal:

*this->left.postfix();
*this->right.postfix();

What is the proper way to do this?

class Node
{
public:
    const char *cargo; 
    int depth; 
    Node *left; 
    Node *right

void Node::postfix()
{
    if (this==__nullptr)
    {
        return;
    }
    else
    {
        *this->left.postfix();
        *this->right.postfix();
        out<<*this->cargo<<"\n";
        return;
    }
};
Peter Stewart
  • 2,857
  • 6
  • 28
  • 30
  • 4
    Why silently return if `this==__nullptr`? Your program has some pretty serious problems if that's ever true – AshleysBrain May 03 '10 at 23:45
  • No, after I implemented the answers I got, it works fine. The recursion (left) procedes until the leaf node is arrived at, then the node is __nullptr, and the next (right) recursive statement is executed. – Peter Stewart May 03 '10 at 23:52
  • 4
    @Peter - although this might, in most cases, work you are actually doing something very bad. Calling a member function on a null pointer is UB. It happens to work in this particular case because most implementations work such that it's something like "postfix(Node*n)" and comparing n==0 would not do anything bad. It doesn't have to work though and may fail sometimes. – Edward Strange May 03 '10 at 23:57
  • 1
    I would caution against using this==__nullptr. Calling member functions of null pointers results in undefined behavior from the C++ spec. I would use if (left != __nullptr) { left->postfix(); } instead – Akusete May 03 '10 at 23:57
  • Instead of out<<*this->cargo<<"\n"; use out<<*this->cargo< – Romain Hippeau May 03 '10 at 23:59
  • 2
    To echo what @Noah says, you need to check whether `left` and `right` point to valid objects before calling member functions on them. Calling the member function and then testing whether `this` is null is backwards and incorrect; it never works correctly, though it can appear to work correctly. – James McNellis May 03 '10 at 23:59
  • To add a further point, the Undefined Behaviour can be observed by making the `postfix()` function `virutal`: `virtual void Node::postfix() { ... }` Most implementations will throw a segmentation fault of some sort when a call is made to a virtual function through a NULL pointer. – Greg Hewgill May 04 '10 at 00:11
  • Thanks for the heads up on calling a member function on a null pointer. I think I'll make this a non-member function. – Peter Stewart May 04 '10 at 00:17
  • 1
    You don't have to make it a non-member function - just use Akusete's suggestion, and check the pointers are not null before calling their members. – AshleysBrain May 04 '10 at 09:44
  • @AshleysBrain, Akusete: yes! thanks very much. I now have a four line function. Now I'm going to optimize the evaluation. I'll ask a separate question on that. Thanks to all for the great help! – Peter Stewart May 04 '10 at 13:33

3 Answers3

10

In a member function, you don't generally need to use this to access class members; you can simply use:

left->postfix();

etc.

If you have function parameters or other local variables that have the same name as a class member variable, you can use this to refer to the member variable, e.g.,

this->left->postfix();

The reason that your code is illegal is that it does not correctly treat left as a pointer. You need to dereference left using -> in order to access its members, as shown in the correct code here. (You could also use the equivalent (*left).postfix(), but that just makes you use more parentheses for no real benefit.)

The use of the indirection operator * at the start of the expression is also incorrect, because it is applied to the result of postfix() (i.e., it dereferences whatever is returned by postfix()). postfix() doesn't return anything, so it's an error. It is important to remember that the . and -> operators both have higher precedence than *.

James McNellis
  • 348,265
  • 75
  • 913
  • 977
6

The -> operator dereferences a pointer, so the extra * will cause problems. You can either do this:

this->left->postfix();

Or this:

(*this).left->postfix();
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
Andy White
  • 86,444
  • 48
  • 176
  • 211
1

*this->left.postfix();

"Dereference the return value of calling postfix() on the 'left' member variable of this."


this->left->postfix();
//or
(*this->left).postfix();

"Call postfix() on the dereferenced 'left' member variable of this."

Edward Strange
  • 40,307
  • 7
  • 73
  • 125