views:

117

answers:

2

This code fills a tree with values based on their depths. But when traversing the tree, I cannot manage to determine the actual number of children without iterating over the parent node. This is necessary because the subleafs are stored in in the node underneath the current one. Which conceptual changes are necessary to store the leafs directly within the current node?

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

#ifndef NULL
#define NULL ((void *) 0)
#endif

// ----

typedef struct _Tree_Node {
    // data ptr
    void *p;

    // number of nodes
    int cnt;
    struct _Tree_Node **nodes;

    // parent nodes
    struct _Tree_Node *parent;
} Tree_Node;

typedef struct {
    Tree_Node root;
} Tree;

void Tree_Init(Tree *this) {
    this->root.p = NULL;
    this->root.cnt = 0;
    this->root.nodes = NULL;
    this->root.parent = NULL;
}

Tree_Node* Tree_AddNode(Tree_Node *node) {
    if (node->cnt == 0) {
        node->nodes = malloc(sizeof(Tree_Node *));
    } else {
        node->nodes = realloc(
            node->nodes,
            (node->cnt + 1) * sizeof(Tree_Node *)
        );
    }

    Tree_Node *res
        = node->nodes[node->cnt]
        = malloc(sizeof(Tree_Node));

    res->p = NULL;
    res->cnt = 0;
    res->nodes = NULL;
    res->parent = node;

    node->cnt++;

    return res;
}

// ----

void handleNode(Tree_Node *node, int depth) {
    int j = depth;

    printf("\n");

    while (j--) {
        printf("    ");
    }

    printf("depth=%d ", depth);

    if (node->p == NULL) {
        goto out;
    }

    int cnt = 0;
    for (int i = 0; i < node->parent->cnt - 1; i++) {
        if (node->parent->nodes[i] == node) {
            cnt = node->parent->nodes[i + 1]->cnt;
        }
    }

    printf("value=%s cnt=%i", node->p, cnt);

out:
    for (int i = 0; i < node->cnt; i++) {
        handleNode(node->nodes[i], depth + 1);
    }
}

Tree tree;

int curdepth;
Tree_Node *curnode;

void add(int depth, char *s) {
    printf("%s: depth (%d) > curdepth (%d): %d\n", s, depth, curdepth, depth > curdepth);

    if (depth > curdepth) {
        curnode = Tree_AddNode(curnode);

        Tree_Node *node = Tree_AddNode(curnode);

        node->p = malloc(strlen(s) + 1);
        memcpy(node->p, s, strlen(s) + 1);

        curdepth++;
    } else {
        while (curdepth - depth > 0) {
            if (curnode->parent == NULL) {
                printf("Illegal nesting\n");
                return;
            }

            curnode = curnode->parent;
            curdepth--;
        }

        Tree_Node *node = Tree_AddNode(curnode);

        node->p = malloc(strlen(s) + 1);
        memcpy(node->p, s, strlen(s) + 1);
    }
}

void main(void) {
    Tree_Init(&tree);

    curnode = &tree.root;
    curdepth = 0;

    add(0, "1");
    add(1, "1.1");
    add(2, "1.1.1");
    add(3, "1.1.1.1");
    add(4, "1.1.1.1.1");
    add(4, "1.1.1.1.2");
    add(4, "1.1.1.1.3");
    add(4, "1.1.1.1.4");
    add(2, "1.1.2");
    add(0, "2");

    handleNode(&tree.root, 0);
}
+1  A: 

I see two problems in you program

1) When you "realloc" the node list, you actually move in memory the node objects, so the parent pointer in their children must me updated as well. I suggest you to transform the array of nodes into an array of pointers to nodes, so you can realloc it without correcting pointers.

2) You forgot to terminate strings:

  node->p = malloc(strlen(s));
  memcpy(node->p, s, strlen(s));

should be:

  node->p = malloc(strlen(s)+1);
  memcpy(node->p, s, strlen(s)+1);

or also simply

  node->p = strdup(s);

Maybe there are other issues, but I strongly suggest to correct these ones first. I hope it may help you Regards

Giuseppe Guerrini
1) Oh, alright. I didn't think of that. Now I understand why Valgrind pointed realloc(). Unbelievable I spent so many hours looking for that bug while looking in the wrong place.2) Sorry, in the real code I wasn't using strings but structs so this wasn't really the culprit.Anyway, the reason I did not go with the array of pointers in the first place was that I feared that they were more inefficient than a large chunk being resized all the time.
+1  A: 

If your structure is truly a tree, then the following pseudo code for recursively counting nodes may help:

def total_number_of_leaf_nodes(node):
    if node does not have children:
        return 1
    else:
        sum = 0
        for each child of node:
            sum += total_number_of_leaf_nodes(child)
        return sum

If it is at all possible for you to use C++, then I would strongly advise it. Being able to use an std::vector or std::list to store your child nodes and being able to make the data element have a template type would greatly simplify the complexity of your code.

Michael Aaron Safyan
Exactly. That's what my remaining problem is. It is down to the part where depth > curdepth is true because it adds a node on the current level and only then goes a level deeper. And that's also the reason why the number of leafs is stored in currentNode->parent->nodes[indexOfCurrentNode + 1]->cnt. How can my current approach be improved to match your 'structure'?