Binary tree (S-Expression) pretty printer

Question

I'm just trying to create a binary tree, and a pretty printer for it:

#include <stdio.h>

typedef unsigned long Leaf;

struct NodeStruct {
    struct TreeStruct * left;
    struct TreeStruct * right;
};
typedef struct NodeStruct Node;

struct TreeStruct {
    char isNode;
    union {
        Node node;
        Leaf leaf;
    } value;
};
typedef struct TreeStruct Tree;

Tree node(Tree* left, Tree* right){
    Tree result;
    result.isNode = 1;
    result.value.node.left = left;
    result.value.node.right = right;
    return result;
};

Tree leaf(unsigned long x){
    Tree result;
    result.isNode = 0;
    result.value.leaf = x;
    return result;
};

void print(Tree* tree, int isRoot){
  if (!tree->isNode){
        printf("%lu",tree->value.leaf);
        return;
    };
    printf("(");
    print(tree->value.node.left,0);
    printf(" ");
    print(tree->value.node.right,0);
    printf(")");
};

int main(){
    Tree a = leaf(1);
    Tree b = leaf(2);
    Tree c = leaf(3);
    Tree d = leaf(4);
    Tree ab = node(&a,&b);
    Tree cd = node(&c,&d);
    Tree abcd = node(&ab,&cd);
    print(&abcd,1);
};

There are so many things that sound wrong to me, yet I don't know how I'm supposed to improve this in C. Ideas?

Also, what if I wanted to return the string instead of printing it?

Answer 1

Disclaimer: I am a C++ coder, but I researched to make sure none of that made it into this advise.

It looks alright, there are a couple little things.

• First, explicitly state that main() takes no arguments. This is applicable to all functions in C:

  int main(void)
  

• leaf should take an argument of Leaf rather than unsigned long, this means that you will only have one line to change if you ever want your tree to hold something else.

  Tree leaf(Leaf x)
  

• Either use <stdbool.h> if on a C99 compiler, or explicitly define true and false rather than just using 1 and 0. This will make your code more self documenting.

  #include <stdbool.h>
  
  #define true 1
  #define false 0
  

• Again, if using <stdbool.h>, use the bool type or roll your own:

  typedef int bool
  

• If you are using C11 then you can use anonymous unions to simplify your TreeStruct. It will also mean you can remove the now superfluous .value throughout your code.

  struct TreeStruct
  {
      bool isNode;
      union
      {
          Node node;
          Leaf leaf;
      };
  };
  

• You don't actually use the isRoot value passed to print, but it can be used to print a newline at the end.

  void print(Tree * tree, bool isRoot)
  {
      if (!tree->isNode)
      {
          printf("%lu", tree->leaf);
          return;
      };
  
      printf("(");
      print(tree->node.left, false);
      printf(" ");
      print(tree->node.right, false);
      printf(")");
  
      if (isRoot)
      {
          printf("\n");
      }
  };
  

• As a matter of personal preference, I like to space things out a bit more, as you can see above.