BST overload assignment operator

[drmario6532]

Hello everyone, I'm currently trying to implement an overload assignment operator for my homework. I'm not really sure where to start and the one that I've tried isn't working (because rhs is readable only). This is what I have so far:

  Node* BSTree::deepCopyHelper(const Node *rhsNode){
    if(rhsNode == nullptr){
        return nullptr; 
    }
    
    Node* copyNode = new Node(); 
    copyNode->_numbers._areacode = rhsNode->_numbers._areacode;
    copyNode->_left = deepCopyHelper(rhsNode->_left);
    copyNode->_right = deepCopyHelper(rhsNode->_right); 
    return copyNode; 
}

BSTree::BSTree(const BSTree& rhs) :_root(deepCopyHelper(rhs._root))
{
}

BSTree& BSTree::operator=(const BSTree& rhs) {
    if(this != &rhs){
    clear(); //Function written already 
    _root = rhs._root; 
    rhs._root = nullptr;  // error: assignment of member ‘DTree::_root’ in read-only object
    }
     return *this;
}

[JRManx]

Your variable rhs is "read-only" because you are passing it as const. So your statement

rhs._root = nullptr;

will not work because you are trying to assign nullptr to a const variable.

Try this:

BSTree& BSTree::operator= (const BSTree& rhs) 
{
    BSTree* ptr = rhs;  // create a new pointer with the same attributes as rhs
    
    if (this != &rhs)
    {
        clear (); //Function written already 
        
        _root = ptr._root; 

        ptr._root = nullptr;
    }
    return *this;
}

Note: I am not 100% sure what your variables are here, so I have no way to test this, but you probably see what I'm getting at. Initialize a local pointer variable with rhs, and use that for doing stuff instead of trying to modify rhs.

Unless, of course, you are trying to modify rhs, in which case ignore all that and pass it by value instead of const reference.

Hope this helps!
max

[dutch]

As usual, agent_max has no idea what he's talking about. Just ignore him.

Your operator= should not just copy the rhs._root pointer. It should do a deep copy. And there is of course no need to set rhs._root to nullptr.

If you are implementing a move assignment operator then you can do something like what you are doing, but it needs to take a BSTree&& instead.

[drmario6532]

Why wouldn't rhs._root be set to nullptr? Wouldn't it leak?

[JRManx]

@dutch,
Ok, I would be the first to admit I'm not an expert on operator overloading, but even I could see that his problem was that he was trying to assign a value (nullptr) to a const (and therefore read-only) variable.

Would you please explain what was wrong with my answer? No offense, but that would be more helpful than just telling me that I have no idea what I'm talking about. Because look, not only do I learn more, but so does the OP. You'd be doing us both a favor!

@drmario6532,
It would help if I knew what exactly rhs._root is. But if it is already a member of a BSTree object, then I think you probably shouldn't set it to null, although that depends on what it is and what it does.

Maybe you could post your members list for the BSTree class, that might help.

Have a good day!
max

@dutch,
How do you know that I'm a he, anyway? LOL ;) ;) ;)

[dutch]

Why wouldn't rhs._root be set to nullptr? Wouldn't it leak?

Why would an assignment destroy the copied object?
If you say x = y, you don't usually expect y to be cleared.
x is not "stealing" y's value. It's copying the value ("copy assignment").
Only if you say x = std::move(y) would you expect y to be cleared (a "move assignment").

Here's a quick'n'dirty example. I wrote this pretty much as quickly as I can type (with a little testing) so it probably has some errors. (And it just uses int for data instead of being a template.)

#include <iostream>
#include <initializer_list>
#include <utility>

class BSTree {
    struct DNode {
        int    data;
        DNode* left;
        DNode* right;
        DNode(int data_in, DNode* left_in=nullptr, DNode* right_in=nullptr)
            : data(data_in), left(left_in), right(right_in) { }
    };

    DNode* root = nullptr;

    static void   clear(DNode*& node);
    static void   print(const DNode* node);
    static void   insert(DNode*& node, int data);
    static DNode* copy(const DNode* node);
public:
    BSTree() = default;
    BSTree(std::initializer_list<int> il) { for (int data: il) insert(data); }
    BSTree(const BSTree& tree) : root(copy(tree.root)) { }
    BSTree(BSTree&& tree) : root(tree.root) { tree.root = nullptr; }
    ~BSTree() { clear(); }
    void clear() { clear(root); }
    void print() { print(root); std::cout << '\n'; }
    void insert(int data) { insert(root, data); }
    void insert(std::initializer_list<int> il) { for (int data: il) insert(data); }
    BSTree& operator=(const BSTree& other); // copy assignment
    BSTree& operator=(BSTree&& other);      // move assignment
};

BSTree& BSTree::operator=(const BSTree& other) {
    if (this != &other) {
        clear();
        root = copy(other.root);
    }
    return *this;
}

BSTree& BSTree::operator=(BSTree&& other) {
    if (this != &other) {
        clear();
        root = std::exchange(other.root, nullptr);
    }
    return *this;
}

BSTree::DNode* BSTree::copy(const DNode* node) {
    return node ? new DNode(node->data, copy(node->left), copy(node->right))
                : nullptr;
}

void BSTree::insert(DNode*& node, int data) {
    if (!node)
        node = new DNode(data);
    else if (data < node->data)
        insert(node->left, data);
    else
        insert(node->right, data);
}

void BSTree::print(const DNode* node) {
    if (!node) return;
    print(node->left);
    std::cout << node->data << ' ';
    print(node->right);
}

void BSTree::clear(DNode*& node) {
    if (!node) return;
    clear(node->left);
    clear(node->right);
    delete node; 
    node = nullptr; 
}

int main() {
    using std::cout;
    
    BSTree t1({5, 3, 0, 7, 4});
    t1.insert({9, 1, 8, 6, 2});
    cout << "  t1: ";
    t1.print();
    
    cout << "Tree t2(t1)\n";
    BSTree t2(t1);
    cout << "  t1: ";
    t1.print();
    cout << "  t2: ";
    t2.print();

    cout << "Tree t3(std::move(t1))\n";
    BSTree t3(std::move(t1));
    cout << "  t1: ";
    t1.print();
    cout << "  t3: ";
    t3.print();

    cout << "t1 = t2\n";
    t1 = t2;
    cout << "  t1: ";
    t1.print();
    cout << "  t2: ";
    t2.print();

    cout << "t1 = std::move(t3)\n";
    t1 = std::move(t3);
    cout << "  t1: ";
    t1.print();
    cout << "  t3: ";
    t3.print();
}

[seeplus]

Assuming that the copy constructor and destructor work OK, then assignment can be as simple as (not tried):

BSTree& BSTree::operator=(BSTree temp) {
    swap(_root, temp._root);    // If only one member variable
    return *this;
}

See https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom

Also similar for move constructor and move assignment.