19

In our company's coding standard, we have been told to "be aware of the ways (accidental) copying can be prevented".

I am not really sure what this means, but assume that they mean we should stop classes from being copied if this is not required.

What I can think of is as follows:

  1. Make the copy constructor of a class private.
  2. Make the assignment operator (operator=) of a class private.
  3. Make the constructor of a class explicit (to stop classes from being created using incorrect variables).
  4. For all classes that carry out memory allocation and where copying is required, make sure that the copy constructor and assignment operator carry out deep copying rather than shallow copying.

Am I on the right track? Is there anything I might have missed out?

Andy
  • 2,770
  • 9
  • 35
  • 42
  • Actually, I would have understood that sentence to mean that you should be aware of these things and watch out that you don't accidentally copy objects you're not supposed to. But what do I know? The question is more interesting this way, anyway. – Tal Pressman Aug 27 '09 at 09:40

5 Answers5

21

Yes, making assignment operator and copy constructor private will prevent you from creating any copy of object using standart methods (but if you really need a copy of an object you can implement, for example, Copy() method, which will perform deep copy).

Take a look on boost::noncopyable.

Update (re to Tal Pressman):

...you should be aware of these things and watch out that you don't accidentally copy objects you're not supposed to.

Well, I presume, that any accidental copy will be performed using either assignment operator or copy constructor. So making them private actually makes sense: if object copying is costly operation, then copying must be explicit: other developer can unintentionally indirectly call copy op and compiler will inform him, that this is forbidden.

11

If you are using boost, then the easiest way to prevent a class from being copied is by deriving your class from noncopyable:

#include <boost/noncopyable.hpp>

class Foo : private boost::noncopyable
{
}

It makes your intention clearer than manually making the copy constructor and assigment operator private, and it has the same result.

Dani van der Meer
  • 6,169
  • 3
  • 26
  • 45
  • I didn't think he wants to prevent the class to be copied, rather he wants to prevent it being copied *accidentally*. – kostia Aug 27 '09 at 09:35
  • I believe he does, because he lists as one of the options making the copy constructor private. – Dani van der Meer Aug 27 '09 at 09:39
  • As much as I like boost, I've run across a number of work situations in which it was simply not allowed. – Greg Oct 06 '11 at 17:22
11

If your coding standard states "be aware of the ways (accidental) copying can be prevented", I'm guessing they aren't just talking about preventing copies from within the classes itself, but about the performance implications of unnecessary / accidental copies when using the classes.

One of the main causes of unnecessarily wasted performance in the code of people new to C++ is unnecessary copying, usually through temporaries. Compilers are getting better and better at deciding when temporaries are not necessary (see "Want speed? Pass by Value", thanks to Konrad's comment), but the best thing to do is to learn to be aware of the inner workings of copying and temporaries in C++ (among others). For me, reading Efficient C++ really got me started.

Joris Timmermans
  • 10,814
  • 2
  • 49
  • 75
  • 2
    Your second point is largely wrong/misleading, when taking copy elision and NRVO into account. To let a professional speak: http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/ – Konrad Rudolph Aug 27 '09 at 11:41
  • That's a great article actually - I was aware of RVO but the last time I delved into it deeply I was still using VS2003 and it was risky business to depend on the compiler to get it right for more complex types. Glad to hear that it is becoming much less of an issue with C++0X – Joris Timmermans Aug 27 '09 at 11:47
  • @MadKeithV: Don’t be mislead by the mention of C++0x: *everything* in that article applies to current compilers and the current version of C++, nothing is new to C++0x. – Konrad Rudolph Aug 27 '09 at 11:49
  • The canonical rule should be "when in doubt, check the assembly-language output" :). – Joris Timmermans Aug 27 '09 at 12:07
2

You are on the right track. If you do not want to use boost you can do the following: Make copy constructor and copy assignment operator private and do not implement them. Thus you will get a compiler error if you try to copy an instance.

h0b0
  • 1,802
  • 1
  • 25
  • 44
0

Your list looks great from the point of view of avoiding errors, e.g. deleting the same area of memory more than once due to shared pointers from implicit copying of objects.

I hope this is also relevant; from the statement "be aware of the ways (accidental) copying can be prevented" you might take this to mean 'be aware of unintentional unnecessary copying'. This would mean circumstances which might affect performance. In a very simple example your coding conventions might mean you should prefer:

std::string text1( "some text" );

over:

std::string text1 = "some text";

The second case would result in a temporary string being created to hold "some text" before the assignment operator is invoked (a form of copying) to populate text1 with "some text". Obviously this is a trivial example and good practice would dictate that you should use the constructor initialisation (first example) where ever possible.

J.Churchill
  • 430
  • 3
  • 12
  • Thank you for your comment. This is something I am not aware of. So always call a constructor rather than use an assignment, if a type conversion is required. – Andy Aug 27 '09 at 10:15
  • I'm not sure you are correct with this one. Both create a temporary char[]. – the_drow Aug 27 '09 at 10:15
  • 2
    No, that’s simply wrong. *All* modern compilers avoid the unnecessary temporary instance of `std::string` in this case. – Konrad Rudolph Aug 27 '09 at 11:41
  • @the_drow - both do create a temporary const char[] but only the second example creates an unnecessary temporary std::string. @Konrad - All modern compilers _should_ avoid it, but not all compilers are modern nor are they required to optimise this away. It's not in the standard as far as I'm aware, but I could be wrong. – J.Churchill Aug 27 '09 at 12:04
  • @J.Churchill: §12.2.2 of the standard explicitly mentions that an implementation may elide the unnecessary copy. All *halfway* modern compilers *do* optimize it. Which probably means all compilers since VC6. So from a purely theoretical standpoint, you’re right. But not in practice. – Konrad Rudolph Aug 27 '09 at 12:13
  • @Konrad - Are there any negatives to using the copy constructor rather than an assignment for std::string initialization? To give a pratical instance, I have worked in the past few years with a C++ compiler and toolchain that has been developed for a specific device architecture and some optimizations allowed by the standard have not been implemented. I'm not sure it is worth the risk to assume that a compiler will take care of it for you when it is easy to prefer constructor initialisation to assignment? – J.Churchill Aug 27 '09 at 12:28
  • @J.Churchill: well, in general both methods are equivalent and using the first over the second has no disadvantages. What makes me wary of the first method is that the syntax is often ambiguous and C++ resolves such ambiguities in favour of function declarations. E.g. consider `int x(int())` which declares a function `x` with return value `int`, taking a single parameter of type “function with return value `int`”. Of course, nobody would write such a contrived declaration but similar cases abound, e.g. when constructing a `string` from `istreambuf_iterator`s to read a whole file into it. – Konrad Rudolph Aug 27 '09 at 14:07