views:

194

answers:

5

I have a Shape class containing potentially many vertices, and I was contemplating making copy-constructor/copy-assignment private to prevent accidental needless copying of my heavyweight class (for example, passing by value instead of by reference).

To make a copy of Shape, one would have to deliberately call a "clone" or "duplicate" method.

Is this good practice? I wonder why STL containers don't use this approach, as I rarely want to pass them by value.

+6  A: 

Restricting your users isn't always a good idea. Just documenting that copying may be expensive is enough. If a user really wants to copy, then using the native syntax of C++ by providing a copy constructor is a much cleaner approach.

Therefore, I think the real answer depends on the context. Perhaps the real class you're writing (not the imaginary Shape) shouldn't be copied, perhaps it should. But as a general approach, I certainly can't say that one should discourage users from copying large objects by forcing them to use explicit method calls.

Eli Bendersky
+1  A: 

Depending on your needs...

If you want to ensure that a copy won't happen by mistake, and making a copy would cause a severe bottleneck or simply doesn't make sense, then this is good practice. Compiling errors are better than performance investigations.

If you are not sure how your class will be used, and are unsure if it's a good idea or not then it is not good practice. Most of the time you would not limit your class in this way.

Brian R. Bondy
+3  A: 

IMHO, providing a copy constructor and assignment operator or not depend more of what your class modelizes than the cost of copying.

If your class represent values, that is if passing an object or a copy of the object doesn't make a difference, then provide them (and provide the equality operator also)

If your class isn't, that is if you think that object of the class have an identity and a state (one also speak of entities), don't. If a copy make sense, provide it with a clone or copy member.

There are sometimes classes you can't easily classify. Containers are in that position. It is meaninfull the consider them as entities and pass them only by reference and have special operations to make a copy when needed. You can also consider them simply as agregation of values and so copying makes sense. The STL was designed around value types. And as everything is a value, it makes sense for containers to be so. That allows things like map<int, list<> > which are usefull. (Remember, you can't put nocopyable classes in an STL container).

AProgrammer
+3  A: 

Generally, you do not make classes non-copyable just because they are heavy (you had shown a good example STL).

You make them non-copyable when they connected to some non-copyable resource like socket, file, lock or they are not designed to be copied at all (for example have some internal structures that can be hardly deep copied).

However, in your case your object is copyable so leave it as this.

Small note about clone() -- it is used as polymorphic copy constructor -- it has different meaning and used differently.

Artyom
+3  A: 

Most programmers are already aware of the cost of copying various objects, and know how to avoid copies, using techniques such as pass by reference.

Note the STL's vector, string, map, list etc. could all be variously considered 'heavyweight' objects (especially something like a vector with 10,000 elements!). Those classes all still provide copy constructors and assignment operators, so if you know what you're doing (such as making a std::list of vectors), you can copy them when necessary.

So if it's useful, provide them anyway, but be sure to document they are expensive operations.

AshleysBrain
I like the examples of STL containers.
Matthieu M.