8

When I compile my class, I get a serious error:

C2558 no copy constructor available or copy constructor is declared 'explicit'

But my copy constructor is neither private nor explicit!

Header:

#include "Csequence.h"

using namespace std;

class Cnoeud
{
private:
    Cnoeud *oNOEpere;
    vector<Cnoeud> oNOEfils;
    Csequence oNOEsequence;
    bool oNOEStatut;

public:
    // Liste des constructeurs
    Cnoeud();
    Cnoeud(Cnoeud &);
    ~Cnoeud(){}

    // Liste des accesseurs et des modificateurs
    Cnoeud * NOEAfficherpere (){ return oNOEpere;}
    vector<Cnoeud> NOEAfficherfils() {return oNOEfils;}
    Csequence NOEAffichersequence() {return oNOEsequence;}
    bool NOEAfficherstatut() { return oNOEStatut;}
    void NOEModifierpere(Cnoeud oNOEp){ *oNOEpere=oNOEp;}
    void NOEModifierfils(vector<Cnoeud>);
    void NOEModifiersequence(Csequence oNOEs){oNOEsequence = oNOEs;}
    void NOEModifierstatut(bool {oNOEStatut = b;}

    // Liste des fonctions membres
    void NOEViderfils(){ oNOEfils.clear();}

    // Surcharge de l'opérateur d'affectation
    Cnoeud & operator=(Cnoeud &) ;
};

Source:

Cnoeud.cpp

#include <iostream>
#include <vector>
#include "Cnoeud.h"

using namespace std;

Cnoeud::Cnoeud()
{
    oNOEStatut= 0;
    oNOEsequence.SEQInitialiserdatesfin(); 
    oNOEsequence.SEQInitialisersequence();
    oNOEpere = NULL;
}

Cnoeud::Cnoeud(Cnoeud & oNOE)
{
    oNOEStatut= oNOE.oNOEStatut;
    oNOEsequence = oNOE.NOEAffichersequence(); 
    oNOEpere = oNOE.NOEAfficherpere();
    oNOEfils.clear();
    vector<Cnoeud>::iterator it;
    for(it=oNOE.NOEAfficherfils().begin();it!=oNOE.NOEAfficherfils().end();it++)
    {
        oNOEfils.push_back(*it);
    }
}
mskfisher
  • 3,291
  • 4
  • 35
  • 48
undertaker705
  • 81
  • 1
  • 1
  • 2
  • Somehow, your code lost all indentation when copy-pasting it from your IDE. It would be far easier to read if you could correct this. There also has to be something missing. `# endif` is just stuck in there randomly, without an `#if`. – Cody Gray - on strike Jan 13 '11 at 10:13
  • To be allowed to use a const declared object's functions, it's functions must be declared as const as well, or the compiler won't know if the functions will alter the const object. I recommend reading Const Correctness [link](http://www.parashift.com/c++-faq-lite/const-correctness.html) on the C++ FAQ site. – Zoomulator Apr 02 '11 at 15:40

3 Answers3

20

That Cnoeud(Cnoeud &) isn't a copy constructor as it is expected by most code using this class. A copy constructor should be

Cnoeud(const Cnoeud &); 

And why wouldn't you make that argument const? A copy constructor certainly shouldn't alter the object it copies from. Note that the same applies to your assignment operator. Also, those accessors should be const member functions:

Cnoeud * NOEAfficherpere () const { return oNOEpere;}
//                          ^^^^^

See this answer about not using const.

Community
  • 1
  • 1
sbi
  • 219,715
  • 46
  • 258
  • 445
  • I did it, but it said thati can't access to getters et setters like bool NOEAfficherstatut() { return oNOEStatut;} Cnoeud * NOEAfficherpere (){ return oNOEpere;} So I add const to these method and after compiling , i found the same error C2558 about the copy contructor – undertaker705 Jan 13 '11 at 10:26
  • 4
    `Cnoeud(Cneoud&);` is a copy constructor. A copy constructor is allowed to take a `const` or a non-`const` reference. – CB Bailey Jan 13 '11 at 10:30
  • @Charles: Oops, I hadn't seen that comment. What do you mean? Should the compiler accept `Cnoeud(Cneoud&)`? – sbi Jan 13 '11 at 14:03
  • 1
    @undertaker705: You did _what_? If that parameter is `const` (as it should), it can only invoke `const` member functions. However, pure accessors should be `const`, too. See [this FAQ](http://stackoverflow.com/questions/4059932/) for how to make member functions declare that they don't change the object they are invoked for. – sbi Jan 13 '11 at 14:05
  • @sbi: I was just saying that a class constructor that takes a non-`const` reference to the class type is still a copy constructor. A compiler shouldn't reject the constructor, but it might have to reject other code that requires the copy constructor to take a `const` reference, for example using the class as a `vector` template argument. "A non-template constructor for class `X` is a _copy_ constructor if its first parameter is of type `X&`, `const X&`, `volatile X&` or `const volatile X&`, and either there are no other parameters or else all other parameters have default arguments (8.3.6)." – CB Bailey Jan 13 '11 at 14:24
  • @Charles: Well, since the OP didn't say where he got that error, and since the code in question isn't compilable, so we can't try for ourselves, it might well be that the error doesn't occur where the class or any of its member functions are defined, but where it is used such that a proper copy constructor is available. Thanks for your insights, I will change my answer to reflect that. – sbi Jan 13 '11 at 15:01
  • 3
    Using VC++ 2008 exp. and got the same error as OP. I hadn't declared the copy argument as const, and VC++ accepted it as a copy constructor after adding it. – Zoomulator Apr 02 '11 at 15:38
3

Copy constructor takes a const reference- that is,

Cnoeud::Cnoeud(const Cnoeud& oNOE)

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • I did it, but it said thati can't access to getters et setters like bool NOEAfficherstatut() { return oNOEStatut;} Cnoeud * NOEAfficherpere (){ return oNOEpere;} So I add const to these method and after compiling , i found the same error C2558 about the copy contructor – undertaker705 Jan 13 '11 at 10:23
  • A copy constructor doesn't _have_ to take a `const` reference although it normally should. In this case it can't because the implementation uses non-`const` member functions of the argument. – CB Bailey Jan 13 '11 at 10:31
2

Cnoeud( Cnoeud & ) is not the normal signature for a copy-constructor although it is sometimes used to "transfer ownership" when copying, i.e. it moves something from the origin to the new object. This is commonly used to get around the issues of which object is going to "clean up" after use when you do not desire to use reference-counted pointers.

With C++0x there will be move semantics with r-value references where you can do Cnoeud( Cnoeud && ) to "move" an object that is not-copyable but can be returned from a function.

It is not safe to use objects with transfer-ownership semantics (like auto_ptr) in collections like vector. You are lucky in a sense that you hit a compiler error because it saved you the pain of a runtime error later - much harder to find the problem.

If you want a proper copy-constructor then pass the parameter as const reference (as other have suggested).

By the way, in your "copy constructor" it appears that

oNOEfils.clear();
vector<Cnoeud>::iterator it;
for(it=oNOE.NOEAfficherfils().begin();it!=oNOE.NOEAfficherfils().end();it++)
{
oNOEfils.push_back(*it);
}

could easily be replaced by

oNOEfils = oNOE.NOEAfficherfils();

More efficient to write (one-liner) and almost certainly more efficient to run too.

CashCow
  • 30,981
  • 5
  • 61
  • 92