HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Unbalanced binary search tree

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unbalancedbinarytreesearch

Problem

I wrote this unbalanced binary tree and would like to know how to improve the code and performance. If you can point out any situations that are not being handled appropriately, that would be great too.

It supports element insertion, removal, search, iteration, tree balancing and encoding/decoding.

bst.h

```
#ifndef BST_H
#define BST_H

#include

//Encoding
#define ENCODING_SIZE_T uint32_t

//Maximum size supported by encoding/decoding
#define MAX_CONTENT_LENGTH ((ENCODING_SIZE_T) -1)

//Return codes
#define BST_SUCCESS 0
#define BST_ERROR 1
#define BST_DUPLICATE 2
#define BST_NO_MEMORY 4

typedef struct BST_Node BST_Node;
struct BST_Node {
void *content;
BST_Node *smaller;
BST_Node *greater;
};

typedef struct {
BST_Node *root;
size_t node_count;
int (compare)(void , void *);
} BST;

static inline void bst_init(BST bst, int (compare)(void , void ))
{
bst->root = NULL;
bst->node_count = 0;
bst->compare = compare;
}

int bst_insert(BST bst, void content);
void bst_remove(BST bst, void content);
void bst_find(BST bst, void *content);
void bst_free(BST *bst);
void bst_iterate(BST bst, void (callback)(void *));
void bst_iterate_reverse(BST bst, void (callback)(void *));
void bst_balance(BST *bst);

/* Return total size of the encoded tree. container will point to the allocated
memory on success. size_after_encoding must return how much space the element
will require after encoded. encoder will be called like encoder(destination,
element, element_size) and should write the encoded element at destination and
return one past it, if it returns NULL, the encoding will be aborted. */
size_t bst_encode
(
BST *bst,
void **container,
ENCODING_SIZE_T (size_after_encoding)(void ), //Must not return 0
void (encoder)(void , void , ENCODING_SIZE_T) //Must return NULL if it fails
);

/* Decode tree. Both constructor and destructor must be set. Constructor is
expected to return a pointer to the decoded e

Solution

This is simple a first pass with mostly top level thoughts - I dig deeper later.

-
Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.

[Edit] I now see bst_init() size_t bst_encode() bst_decode() are only prototyped in bst.h - ignore that part of this suggestion. At first blush, the style looked like code, but it is only prototype.

// BST.h
typedef struct BST BST;`

// BST.c
struct BST { 
  BST_Node *root; 
  size_t node_count; 
  int (*compare)(void *, void *);
};


-
I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

-
Need #include for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

-
Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

-
Change compare function type to a const version. qsort()'s type shown for reference.

int (*compare)(const void *, const void *); 
// qsort's --> int (*compar)(const void *, const void *);


-
Maybe test for compare() against NULL.

-
To expand compare() possibilities, add a compare state variable.

-
Concerning callback, allow that function to return an int and use a non-zero value to short-circuit the progress OR provide a state variable for callback().

More later

Code Snippets

// BST.h
typedef struct BST BST;`

// BST.c
struct BST { 
  BST_Node *root; 
  size_t node_count; 
  int (*compare)(void *, void *);
};
int (*compare)(const void *, const void *); 
// qsort's --> int (*compar)(const void *, const void *);

Context

StackExchange Code Review Q#39282, answer score: 4

Revisions (0)

No revisions yet.