4

I need to implement a void function that computes the height of each node in a binary tree and stores it in each node. I've found a few solutions online that are recursive in nature but they return int. Examples include (https://www.geeksforgeeks.org/write-a-c-program-to-find-the-maximum-depth-or-height-of-a-tree/). The difference between the model answer, besides that it is not a void function, is that it also does not store the height in each node.

This is my attempt at the solution, but I can't seem to get the code to work, nor refit the model answer to recursively apply in a void function. When I run my code in the helper code to test, it doesn't even show any output.

void computeHeight(Node *n) {
  Node* ltraverser = n;
  Node* rtraverser = n;
  int lheight = 0;
  int rheight =0;

  if (n == NULL) {
   n->height = 0;
  }

  while (ltraverser->left != NULL) {
   ltraverser = ltraverser->left;
   lheight += 1;
  }

  while (rtraverser->right != NULL) {
   rtraverser = rtraverser->right;
   lheight += 1;
  }

  if (lheight > rheight) {
    n->height = lheight;
  }

  else {
    n->height = rheight;
  }

  computeHeight(n->left);
  computeHeight(n->right);
}

For reference:

The starter code below defines a class called "Node" that has two child pointers ("left" , "right") and an integer "height" member variable. There is also a constructor Node() that initializes the children to nullptr and the height to -1.

/*
The height of a node is the number of edges in
its longest chain of descendants.

Implement computeHeight to compute the height
of the subtree rooted at the node n. Note that
this function does not return a value. You should
store the calculated height in that node's own
height member variable. Your function should also
do the same for EVERY node in the subtree rooted
at the current node. (This naturally lends itself
to a recursive solution!)

Assume that the following includes have already been
provided. You should not need any other includes
than these.

#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <string>

You have also the following class Node already defined.
You cannot change this class definition, so it is
shown here in a comment for your reference only:

class Node {
public:
  int height; // to be set by computeHeight()
  Node *left, *right;
  Node() { height = -1; left = right = nullptr; }
  ~Node() {
    delete left;
    left = nullptr;
    delete right;
    right = nullptr;
  }
};
*/

For testing the code

// This function prints the tree in a nested linear format.
void printTree(const Node *n) {
  if (!n) return;
  std::cout << n->height << "(";
  printTree(n->left);
  std::cout << ")(";
  printTree(n->right);
  std::cout << ")";
}

  Node *n = new Node();
  n->left = new Node();
  n->right = new Node();
  n->right->left = new Node();
  n->right->right = new Node();
  n->right->right->right = new Node();

  computeHeight(n);

  printTree(n);
  std::cout << std::endl << std::endl;
  printTreeVertical(n);

  delete n;
  n = nullptr;

  return 0;
}
Dumb chimp
  • 444
  • 1
  • 4
  • 13
  • how do you expect to get the result out of the function? If it does not return anything you need an out parameter. – 463035818_is_not_an_ai Oct 21 '19 at 18:19
  • I believe the assignment specifies that I should store the calculations in n->height, but I'm as bamboozled as you. @formerlyknownas_463035818 – Dumb chimp Oct 21 '19 at 18:23
  • Unrelated. I looked at the Geeks for Geeks link for the code sample you were basing on. Use caution with [`#include `](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) and [`using namespace std;`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). Use a lot more caution when combining the two like the example does. – user4581301 Oct 21 '19 at 18:23
  • 2
    @ This code snippet if (n == NULL) { n->height = 0; } invokes undefined behavior. – Vlad from Moscow Oct 21 '19 at 18:29
  • On topic, I'd make a `getheight` accessor member function of `Node` that does the heavy lifting by recursing to the end of the tree and returning the height all the way back. Damn shame you're not allowed to change `Node`. Such is life. `computeHeight` returns nothing, but no one said (or maybe they did, but you didn't) you can't call a more useful function from `computeHeight`. – user4581301 Oct 21 '19 at 18:31
  • The recursive function would use the greatest hight found in another parameter (passed by reference, so it can be updated). Also, each node may have two height parameters, updated in each call. This function must be called in every node, I mean, don't use right-branch-only or left-branch-only calls. – Ripi2 Oct 21 '19 at 18:32
  • @user4581301 I would love to try, I did actually attempt to call a function from inside computeHeight, but failed so miserably. – Dumb chimp Oct 21 '19 at 18:33
  • @Ripi2 I'm sorry, I don't quite get it. – Dumb chimp Oct 21 '19 at 18:36
  • 1
    Being in the root node or in any other node is the same: you have to find the depth from the current node. Once you found it, you pass this info to the "parent" node. Because the parent node asked its children to do the job, each child stores the info in the passed argument. Storing both l-depth, r-depth is an additional info which can be useful in future queries. – Ripi2 Oct 21 '19 at 18:41
  • BTW the question is taken from this excellent course https://www.coursera.org/learn/cs-fundamentals-2 – chris Nov 23 '20 at 17:48

2 Answers2

7

Instead of returning node height just recurisvely call computeHeight on left and right nodes, then store maximum height in node structure.

#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <string>
#include <algorithm>

class Node {
public:
  int height;
  Node *left, *right;
  Node() { height = -1; left = right = nullptr; }
  ~Node() {
    delete left;
    left = nullptr;
    delete right;
    right = nullptr;
  }
};

void computeHeight(Node *node) {

    if (node == nullptr) {
        return;
    }

    computeHeight(node->left);
    computeHeight(node->right);

    int leftHeight = -1;
    int rightHeight = -1;

    if (node->left != nullptr) {
        leftHeight = node->left->height;
    }

    if (node->right != nullptr) {
        rightHeight = node->right->height;
    }

    node->height = std::max(leftHeight, rightHeight) + 1;
}

void printNode(Node *n, int level = 0) {
    if (n == nullptr) {
        return;
    }

    std::cout << std::string(level * 2, ' ') << "Height = " << n->height << "\n";

    printNode(n->left,  level + 1);
    printNode(n->right, level + 1);
}


int main() {
  Node *n = new Node();
  n->left = new Node();
  n->right = new Node();
  n->right->left = new Node();
  n->right->right = new Node();
  n->right->right->right = new Node();


  computeHeight(n);
  printNode(n);
}
Edin Omeragic
  • 1,948
  • 1
  • 23
  • 26
3

Your mistake is on the following part and because of this you program exits without showing the error

if (n == NULL) {
   n->height = 0;
}

When n is NULL; you should not try to access n->height. Replace it as follows and your code will work:

if (n == NULL) {
    return;
}

Also, as the other answer mentioned, when you want to compute height recursively, you don't need a while loop just use the following recursive formula:

Height(n) = 1 + max(Height(n->left), Height(n->right))

Also, for consistency reasons usually the height of NULL subtree is defined to be -1. This allows the recursive formula to work properly.

Word of advice: In order to debug any program, an easy way is to just print messages before and after function calls and/or certain lines. This way by checking which messages are not printed, you can quickly pinpoint which functions/lines are causing a problem and then investigate them.

A. Mashreghi
  • 1,729
  • 2
  • 20
  • 33