1

I am using some of the vendor libraries in my code. Some of the codes are like below. I see it uses circular dependency between classes.I am unable to understand what is the reason of making the copy constructor and assignment as private. One of the static code analysis tool throws violation as "Avoid classes using 'new' to allocate instances but not defining a copy constructor."

class Parent;

class Child{
public:
Child(Parent& parent):mrParent(parent);

private:
Parent& mrParent;
};

class Parent{
public:
Parent();
~Parent();
//other declarations
Child* child;
private:
//copy and assignment are not allowed. Explicitly declaring private
Parent(const Parent&)
Parent& operator=(const Parent&);
};
gsamaras
  • 71,951
  • 46
  • 188
  • 305
user1706047
  • 369
  • 3
  • 7
  • 19
  • 3
    the reason is in the comment: `//copy and assignment are not allowed. Explicitly declaring private` – Jean-François Fabre Sep 18 '17 at 12:05
  • also, don't check vendor libraries you cannot change in a rulecheck tool. – Jean-François Fabre Sep 18 '17 at 12:06
  • Possible duplicate of [What's the use of the private copy constructor in c++](https://stackoverflow.com/questions/6811037/whats-the-use-of-the-private-copy-constructor-in-c) – Jean-François Fabre Sep 18 '17 at 12:13
  • I think this static code analysis tool emits incorrect diagnostic. The problem reported would be relevant if class had default copy constructor and /or assignment. Basically it looks like that tool didn't recognize that these constructors can not be used so no issue occurs. Copy constructors and assignment should be private / deleted unless class instances are explicitly required to be copyable. – user7860670 Sep 18 '17 at 12:16
  • It would be interesting to know which static analysis tool the OP is using. – roalz Sep 18 '17 at 12:18

3 Answers3

2

What you have is a common way of avoiding copy construction and assignment to a class.
That is the intent of whomever wrote that code, as stated in the comment:

//copy and assignment are not allowed. Explicitly declaring private

Because both copy constructor and assignment operator are declared but not implemented, this may lead static analysis tools to issue a warning.

In C++11, a better way to deny copy construction and assignment is:

Parent(const Parent&) = delete;
Parent& operator=(const Parent&) = delete;

this should also avoid the warning from the static analysis tool (if not, the tool is probably broken).

roalz
  • 2,699
  • 3
  • 25
  • 42
  • thanks for the explanation. I was trying to understand what is the need of avoiding copy constructor and assignment. I understand as per the comment but i m not sure abt the reason. Also regarding c++11 , "= delete" here i am getting compilation error as bad syntax for pure function definition. Is it like my compiler doesnt support C++11? – user1706047 Sep 18 '17 at 12:30
  • C++11: depending on the compiler you use, if it's a fairly recent MSVC, it supports it, if it's GCC you may need to "force" it to use c++11 (commandline option --std=c++11) – roalz Sep 18 '17 at 13:19
1

I am unable to understand what is the reason of making the copy constructor and assignment as private.

The developer does not want the user of the class to be able to copy or/and assign instances of this class.

For that reason they do not provide that functionality, which can be done by declaring them at private scope.

The comment in the code is rather desacriptive:

//copy and assignment are not allowed. Explicitly declaring private
gsamaras
  • 71,951
  • 46
  • 188
  • 305
1

The warning received from the static analysis is a false positive: the copy constructor is not "not-defined" but is explicitly denied.

If the copy constructor was really "not defined" then the compiler would have generated one that would not have worked as intended (because of the new operator).

However, since the copy constructor has been explicitly denied then the compiler will not generate any default copy constructor on its own.

Paolo Brandoli
  • 4,681
  • 26
  • 38
  • 1
    It is only declared, not defined (at least in this snippet). Even it were defined, I think you're taking the warning pedantically literally (although it could do with being more precise); it seems to mean "Avoid classes using 'new' to allocate instances but not defining a copy constructor [accessible whereever new instances of this class may be created]". Why it has that warning is another matter. – Arthur Tacca Sep 18 '17 at 12:26