0

To calculate the height of a binary tree, height(left)+height(right)+1 is used and the height of non-existent nodes are defined as -1. So, if left or right is null, then the formula works without specifying these cases explicitly.

I implemented this idea in C++, and I ensured that the member functions will NOT access member variables if this == nullptr. My implementation is successful and works (at least on my Linux machine).

I want to find out what kind of practical downsides this approach can have, and if the standard stipulates anything about it. So, that's my question.

#include <algorithm>
#include <iostream>

struct Node {
    // default values of the child nodes are NULL
    Node *left = nullptr;
    Node *right = nullptr;

    // store some value
    int v;

    int get_height() const __attribute__ ((optimize(0))) {
        if (this == nullptr)
            return -1;
        return std::max(left->get_height(), right->get_height()) + 1;
    }
};

int main() {
    Node node1, node2, node3, node4;
    node2.left = &node1;
    node2.right = &node3;
    node3.right = &node4;
    std::cout << node2.get_height() << std::endl;
    return 0;
}

Edit: If optimization are enabled the code fails with a segmentation fault. It is possible to fix the problem by appending __attribute__ ((optimize(0))) to the function.

jnbrq -Canberk Sönmez
  • 1,790
  • 1
  • 17
  • 29
  • 5
    There is no real reason to do the check. If it is nullptr, we're already in UB land – AndyG Sep 04 '19 at 21:57
  • 1
    It is a pretty decent way to detect a bug in the caller of your function. Not terribly compatible with C++ sensibilities however, users of your code that you don't know and are unlikely to be skilled programmers are never allowed to get it wrong. You have to always be ready to take the phone call, such is the price of being the smart guy. Do beware it isn't necessary nullptr when you have a base class. – Hans Passant Sep 04 '19 at 22:05

4 Answers4

8

this == nullptr is "safe" in the sense that it has no side-effects whatsoever.

this == nullptr is useless in the sense that in any program with well defined behaviour, it is never true. As such, the optimizer is allowed to pretend that you had instead written:

if (false)
    return -1;
return std::max(left->get_height(), right->get_height()) + 1;

Which is same as having written:

return std::max(left->get_height(), right->get_height()) + 1;

What isn't safe is calling a member function through a null pointer (or any other pointer that doesn't point to an object with active lifetime). This check doesn't protect against that, even though one might intuitively think that it does. The behaviour of the example program is undefined.

An example of well defined implementation of the function:

int get_height() const {
    int lh = left  ? left->get_height()  : -1;
    int rh = right ? right->get_height() : -1;
    return std::max(lh, rh) + 1;
}

A non-member function may be useful in avoiding the little bit of repetition.


P.S. A good compiler will warn you about the potential mistake:

warning: 'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false [-Wtautological-undefined-compare]

    if (this == nullptr)
        ^~~~    ~~~~~~~
Community
  • 1
  • 1
eerorika
  • 232,697
  • 12
  • 197
  • 326
2

No, if (this == nullptr) is not "safe". The only way the if statement could be true is if you called the member function on a null pointer and doing that is undefined behavior. That means the check is completely superfluous as it can never be true in legal code.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
1

this cannot be nullptr and compiler likely would optimize it out, because this being a null pointer is result of UB.

Similar case:

  ptr->someStaticMethod();   // if ptr is null this is an UB, even if method is static.

  if(!ptr) 
      abort();   // May never happen, even if ptr is nullptr
Swift - Friday Pie
  • 12,777
  • 2
  • 19
  • 42
0

If you would like to be able to deal with nullptr while attempting to get the height of an object, you should use a non-member function.

namespace MyApp
{
   int get_height(Node const* n)
   {
      return ( n == nullptr ) ? -1 : n->get_height();
   }
}

and use

Node* n = ...;

...

int h = MyApp::get_height(n);
R Sahu
  • 204,454
  • 14
  • 159
  • 270