0

NB: This question follows a previous one, I hope it is okay to still ask it as a new question.

I am trying to implement the "three and a half big rule" (copy-and-swap idiom) for a tree class, which looks like this:

class Tree
{
    friend void swap(Tree &first, Tree &second); // Swap function

public:
    Tree(const double &a, const double &b, int depth); // Public constructor (derived from the default (private) constructor)
    Tree(const Tree &other); // Copy constructor
    ~Tree(); // Destructor
    Tree & operator=(Tree other); // Copy-assignement operator


private:        
    Tree(double *a, double *b, int depth, int maxDepth); // Default (private) constructor

    double *a, *b;
    int depth, maxDepth;    
    Tree *leftChild, *rightChild;
};

I have been trying to follow this guideline. Here is what my copy-assignment operator looks like:

Tree & Tree::operator=(Tree other)
{
    swap(*this, other);
    return *this;
}

I am having a hard time getting my public constructor to work. Someone suggested I do something like:

Tree::Tree(const double &a, const double &b, int depth)
{
    double aTemp(a), bTemp(b);
    swap(*this, Tree(&aTemp, &bTemp, depth, depth));
}

I am not sure this idea works. In any case I get the following error from the compiler:

invalid initialization of non-const reference of type 'Tree&' from an rvalue of type 'Tree'
in passing argument 2 of 'void swap(Tree&, Tree&)'

I tried the following idea instead, which I thought would work:

Tree::Tree(const double &a, const double &b, int depth)
{
    double aTemp(a), bTemp(b);
    *this = Tree(&aTemp, &bTemp, depth, depth);
}

But it does not seem to be working either. I think the problem is that when I call the copy-assignment operator (*this = Tree(&aTemp, &bTemp, depth, depth)), the copy constructor should be called (since the argument of the copy-assignement operator is passed by value), but it seems that this is not happening. I do not understand why.

Thanks in advance for helping!

Community
  • 1
  • 1
Seub
  • 2,451
  • 4
  • 25
  • 34

1 Answers1

1

invalid initialization of non-const reference of type 'Tree&' from an rvalue of type 'Tree' in passing argument 2 of 'void swap(Tree&, Tree&)'

C++ does not allow passing anonymous objects by non-const reference. The intent is to prevent callers from accidentally throwing away the results of functions that write to a reference argument.

You instead could do:

Tree::Tree(const double &a, const double &b, int depth)
{
    double aTemp(a), bTemp(b);
    Tree temp(&aTemp, &bTemp, depth, depth);
    swap(*this, temp);
}

But it does not seem to be working either. I think the problem is that when I call the copy-assignment operator (*this = Tree(&aTemp, &bTemp, depth, depth)), the copy constructor should be called (since the argument of the copy-assignement operator is passed by value), but it seems that this is not happening. I do not understand why.

How are you determining that it's not working? The compiler might elide away the copy to avoid doing unnecessary work. (That is why your copy-assignment operator takes the argument by value.)

BTW, if your compiler supports C++11, you could use delegating constructors instead.

Community
  • 1
  • 1
jamesdlin
  • 81,374
  • 13
  • 159
  • 204
  • Thanks for your answer. I'm not familiar with copy elision, I'll look into it (turns out using a temporary variable as you do but with the "second solution" does make the compiler call the copy constructor). So I guess `*this = Tree(&aTemp, &bTemp, depth, depth);` might work after all. I'll think about which solution I go for. As for the "delegating constructors" (which I hadn't heard of until now), I don't see how I could make use of that here because of the `const` qualifiers (in `Tree(const double &a, const double &b, int depth)`) – Seub Aug 09 '13 at 05:24
  • Right, you wouldn't be able to use delegating constructors directly in the current form, but possibly you could rewrite your code around them. (Currently it's not clear why you bother having the private `Tree` constructor that takes non-`const double*` arguments.) – jamesdlin Aug 09 '13 at 05:58