-1

There are two integers x and 7 which are randomly generated integers. The program uses a red black tree member fucntion insert to insert new values into the tree.

I'm not understand the arguments of the insert function, more specifically the use of

(void*)x and (void*y) 

Here's the function call in main

rbt.rbtree_insert(t, (void*)x, (void*)y, compare_int);

Here's the insert function defined

void RBTree::rbtree_insert(rbtree t, void* key, void* value, compare_func compare)
{
    node inserted_node = new_node(key, value, RED, NULL, NULL);
    if (t->root == NULL)
    {
        t->root = inserted_node;
    }
    else
    {
        node n = t->root;
        while (1)
        {
            int comp_result = compare(key, n->key);
            if (comp_result == 0)
            {
                n->value = value;
                return;
            }
            else if (comp_result < 0)
            {
                if (n->left == NULL)
                {
                    n->left = inserted_node;
                    break;
                }
                else
                {
                    n = n->left;
                }
            }
            else
            {
                assert(comp_result > 0);
                if (n->right == NULL)
                {
                    n->right = inserted_node;
                    break;
                }
                else
                {
                    n = n->right;
                }
            }
        }
        inserted_node->parent = n;
    }
    insert_case1(t, inserted_node);
    verify_properties(t);
}
Daniel R
  • 103
  • 1
  • 5
  • I take it this is not your code. You should not use `void*` in C++ as we have templates. What you are seeing is called [explicit casting](http://en.cppreference.com/w/cpp/language/explicit_cast) – NathanOliver Dec 21 '15 at 14:50
  • What is it you don't understand? It's a generic tree storing `void*`, which was common practice pre-templates and still is in C. (I wouldn't be surprised if that code has been "ported" from C by wrapping it in a thin layer of C++ classes.) – molbdnilo Dec 21 '15 at 14:51

2 Answers2

0

void* is a type. More specifically, it is a pointer type. Even more specifically, it is a special pointer type that can point to any type. void* is a way to implement polymorphism in C. It's usually not recommended to use void* in C++.

(void*)x is an explicit type conversion also known as C-style type cast expression. The type of variable x is converted to void*. The use of C-style casts is discouraged in C++.

Presumably the type of x is not void* and therefore a conversion is needed to match the type of the argument.

eerorika
  • 232,697
  • 12
  • 197
  • 326
0

The author of this code uses the most abstract pointer type available in C++: void*. It is a pointer so "something". What this "something" might be is not defined at compile time. (void*)x is a type cast in legacy C-syntax, that interprets any other pointer as void*. The prefered C++ syntax is static_cast<void*>(x), though void* should only be used, when there are very, very good reasons to do so.

I understand, that this is legacy code, you have been asked to work on. So let's be frank, there is quite a lot wrong with it.

  1. There are exactly two reason to implement a red-black-tree class: learning data structures and std::map<> as part of a standard library implementation. In all other cases, there is no reason not to prefer std::map<>. It'd save you from all the design pitfalls, the author of this code has stepped in.

  2. It is redundant to add the name of the class to the name of a member function. Call it RBTree::insert() instead of RBTree::rbtree_insert(). When you use consistent member function names for different container types, you can easily exchange them in future, without having to change all the calls to all the member functions. The standard containers are a good source of inspiration here.

  3. A red-black-tree instance always has to work with the same compare function. To pass the compare function again and again to insert(), find(), erase(), etc. is not only redundant but also error prone. Either have it as a parameter to the constructor, or better as a template parameter to a red-black-tree class template.

  4. In any case a red-black-tree should be a template, that has a key and a value type as template parameters. Then all the member functions like insert(), find(), etc. can be typesafe.

  5. Why should one pass the this object explicitly to a member function. I guess, the author of that code has been trying to write Python in C++. In C++ this is always implicit for member functions, in contrast to self in Python.

Putting it all together I'd propose an interface like this:

template<typename key_t,
         typename value_t,
         typename Compare = std::less<key_t>>
class rb_tree {
    void insert(const key_t& key, const value_t& value);
    void erase(const key_t& key);
    value_t find(const key_t& key);
};

As you see, we now define types for keys and values and use them in insert(), erase() and find(). These functions can never try to walk a tree with int keys as if it had std::string keys. They also always use the same compare function, which defaults to the operator <.

The usage is a lot better to understand as well:

rb_tree<int, int> rbt; // we use the default comparison
int x = 42;
int y = 4711;
rbt.insert(x, y);
assert(rbt.find(x) == y);
rbt.erase(x);

Well, actually my real suggestion is to drop than homegrown red-black-tree and use std::map<> instead. Its usage is even more intuitive:

std::map<int, int> rbt;
int x = 42;
int y = 4711;
rbt[x] = y;
assert(rbt[x] == y);
rbt.erase(x);
cdonat
  • 2,748
  • 16
  • 24