Thread: copy constructor doesn't invoke

I see I can't edit my las replay so I am writing in a new one.
I want to emphasize the problem so here is a piece of code of the idea:

Code:

// say somewhere in the code I have this assignment:
RBNode* y;
y = some_other_node_pointer; // when some_other_node_pointer points to nil!
// sure now y points to our static nil instance...

// then there might be some thing like that:
y->right = x; // x is also RBNode* and isn't nil

As you can see, problem is, I change my nil which should not be changed at all.
Fix me if I am wrong but it seems to me that the best way to deal with it is to do something like that:

Code:

RBNode* y;
y .operator=(some_other_node_pointer); 
y->right.oprator = (x); 

// operator = will make sure the argument isn't nil. If it's nil it will allocate another instance and will initialize it as I want..

Make a helper function to deal with that. Operators are just syntactic sugar.

Originally Posted by Elysia

Make a helper function to deal with that. Operators are just syntactic sugar.

Yeah, I'm calling it "operator =" just for the visibility of the code it's still a method...

Thanks.

Originally Posted by gavra

However, I don't see any other way here. Pay attention to the fact that I have three rb trees in my program, each with a diffrent node type (CNode for a customer node, BookNode and CountsNode - all that is needed inorder to ensure a logarithmic complexity for every request in the assignment).

There are a number of other ways:

1. The absolute best way would be to make this a template class. E.g. rather than RBNode, you'd have RBNode<T>, which you'd use as RBNode<Customer>, etc. Here you'd store a T or a T* inside the node.

2. Or, you can make RBNode have a pointer to a class called something like RBData, which is an abstract class with a virtual destructor. Make Book, Customer, etc. derive from RBData.

3. Lastly (though you need to be careful about how you destroy node data) you could fall back to storing a void * inside your tree. That pointer might, in different trees, point to either a Book, or a Customer, etc.

I'll just add:
- #1 is the best option in terms of performance, but typically has the most complexity.
- #2 can have a slight performance overhead due to virtual functions. The compiler has to deduce the proper function to call at runtime and hence cannot inline functions.
- #3 is perhaps the worst option of all. Removing type information is not recommended in C++, so use it with care, and only after you've considered the other alternatives.

Originally Posted by Elysia

I'll just add:
- #1 is the best option in terms of performance, but typically has the most complexity.
- #2 can have a slight performance overhead due to virtual functions. The compiler has to deduce the proper function to call at runtime and hence cannot inline functions.
- #3 is perhaps the worst option of all. Removing type information is not recommended in C++, so use it with care, and only after you've considered the other alternatives.

Yup, I agree completely with that analysis. Thanks for adding that - I should have more clearly laid out the pros and cons with each approach.

Originally Posted by Cat

There are a number of other ways:

1. The absolute best way would be to make this a template class. E.g. rather than RBNode, you'd have RBNode<T>, which you'd use as RBNode<Customer>, etc. Here you'd store a T or a T* inside the node.

2. Or, you can make RBNode have a pointer to a class called something like RBData, which is an abstract class with a virtual destructor. Make Book, Customer, etc. derive from RBData.

3. Lastly (though you need to be careful about how you destroy node data) you could fall back to storing a void * inside your tree. That pointer might, in different trees, point to either a Book, or a Customer, etc.

Originally Posted by Elysia

I'll just add:
- #1 is the best option in terms of performance, but typically has the most complexity.
- #2 can have a slight performance overhead due to virtual functions. The compiler has to deduce the proper function to call at runtime and hence cannot inline functions.
- #3 is perhaps the worst option of all. Removing type information is not recommended in C++, so use it with care, and only after you've considered the other alternatives.

Thank you both.
I think I'll go with the second option, seems more convinient to implement.
If I get it straight, by moving the extra data to a memeber in RBNode, I don't need to worry about the nil problem.
How do you think I should treat the nil here? I still think I'm gonna need to declare it as static.

Really, go with nil being nullptr. Is see no advantage to having a static member. You are working with pointers, so you don't need to "invent" a null value. There already is one.

Originally Posted by gavra

If I get it straight, by moving the extra data to a memeber in RBNode, I don't need to worry about the nil problem.

The main reasons to move to one of these options:

1. Better separation of concerns. You reduce the coupling between the tree structure and the tree data. One class can focus purely on creating and maintaining the structure of the tree, and the others can focus purely on the data they contain. Your data nodes don't need to care about tree logic, and your tree doesn't need to care about its payload.

2. Much easier to code the tree itself, because you can add new nodes to the tree without having to worry about which subclass of node you need to construct. That is, you don't need to figure out if you need to call new BookNode() or new CustomerNode() etc, you just create a new RBNode and put whatever data you are given into it.