which is better: a lying copy constructor or a non-standard one?

Question

I have a C++ class that contains a non-copyable handle. The class, however, must have a copy constructor. So, I've implemented one that transfers ownership of the handle to the new object (as below)

class Foo
{
public:
    Foo() : h_( INVALID_HANDLE_VALUE )
    {
    };

    // transfer the handle to the new instance
    Foo( const Foo& other ) : h_( other.Detach() )
    {
    };

    ~Foo()
    {
        if( INVALID_HANDLE_VALUE != h_ )
            CloseHandle( h_ );
    };

    // other interesting functions...

private:

    /// disallow assignment
    const Foo& operator=( const Foo& );

    HANDLE Detach() const
    {
        HANDLE h = h_;
        h_ = INVALID_HANDLE_VALUE;
        return h;
    };

    /// a non-copyable handle
    mutable HANDLE h_;
}; // class Foo

My problem is that the standard copy constructor takes a const-reference and I'm modifying that reference. So, I'd like to know which is better (and why):

1. a non-standard copy constructor: Foo( Foo& other );

2. a copy-constructor that 'lies': Foo( const Foo& other );

Thanks, PaulH

---

Edit:

DuplicateHandle() only works on specific types of handles. This is not one of them. This handle cannot be duplicated, copied, or cloned.

Several people have pointed out that I was mistaken in suggesting it is a non-standard copy constructor and that std::auto_ptr does this. I think that's probably the way to go. But, I end up with warnings every time I use the class when I make the copy ctor take a non-const value. For example:

namespace detail {
    class Foo { ... };
};

class Buzz
{
public:
    typedef detail::Foo Fuzz;

    Fuzz bar() const { return Fuzz(); }; // warning here
};

warning C4239: nonstandard extension used : 'argument' : conversion from 'Foo' to 'Foo &'
1> A non-const reference may only be bound to an lvalue; copy constructor takes a reference to non-const

Can anybody suggest what I should do about them?

Thanks, PaulH

---

Edit2:

Everybody seems to be steering me towards std::auto_ptr<>'s method of doing things. So, I looked there and it uses an intermediate structure to get around the issue I described in the first edit. This is the solution I came up with.

class Foo;

struct Foo_ref
{
    explicit Foo_ref( Foo& other ) : ref_( other ) {};
    Foo& ref_;
private:
    const Foo_ref& operator=( const Foo_ref& );
}; // struct Foo_ref

class Foo
{
public:
    Foo() : h_( INVALID_HANDLE_VALUE )
    {
    };

    // transfer the handle to the new instance
    Foo( Foo_ref other ) : h_( other.ref_.Detach() )
    {
    };

    ~Foo()
    {
        if( INVALID_HANDLE_VALUE != h_ )
            CloseHandle( h_ );
    };

    operator Foo_ref()
    {
        Foo_ref tmp( *this );
        return tmp;
    };

    // other interesting functions...

private:

    /// disallow assignment
    const Foo& operator=( const Foo& );

    HANDLE Detach()
    {
        HANDLE h = h_;
        h_ = INVALID_HANDLE_VALUE;
        return h;
    };

    /// a non-copyable handle
    HANDLE h_;
}; // class Foo

It compiles cleanly on warning level 4 and seems to work. Please, let me know if it is somehow more irresponsible than my original post.

Thanks, PaulH

Answer 1

The first option has a well established precedent in the form of auto_ptr:

http://www.cplusplus.com/reference/std/memory/auto_ptr/auto_ptr/

auto_ptr gives up its pointer and is reset when it is copied.

The standard that ensures that a function doesn't change arguments passed as const is much stronger than the copy-ctor standard which is really not very formal.

Also, manipulating a const value by casting away its constness is undefined behavior according to the standard. You can do it if either you are certain that the reference refers to a non-const object or you are passing a const object to a (const-incorrect) function that will not modify the value.

Answer 2

I definitely would not implement the copy constructor as you have here. The Detach method is declard as const but really isn't. It would be better to make the copy constructor follow the standard form but make it private and/or make it throw on invocation. Then, you could provide a Clone method that duplicates the handle and any other applicable attributes.

Answer 3

That's easy: a non-standard one. A constructor that modifies its const parameter would be very difficult to predict by the developer. You'd need it to be documented and the developer would need to know to look at the documentation for a very common use....something most won't do.

A non-standard constructor though begs attention.

Besides, this is how the standard does it with auto_ptr. Why? So you can't accidentally put it in a container. Another good reason to use the non-standard version.

Answer 4

Your class is basically doing what std:;auto_ptr does. The copy constructor for auto_ptr looks like:

auto_ptr( auto_ptr & p );

which means it cannot be used in certain situations, like in standard containers. This is probably best practice, but its noticeable that auto_ptr has been deprecated in C++0x.

Answer 5

Maybe the copy constructor should just use DuplicateHandle() to make a real copy.

Answer 6

Well, if you truly need a copy constructor, then you must ensure that it has proper semantics. If you implement either of your approaches, then you're asking for trouble. Code that depends on your copy constructor will not know about your transfer of ownership scheme, and this can have unpredictable results. (This is the reason, for example, that you can't use a std::auto_ptr in an STL container.)

If you don't really need a copy constructor, then create a method that copies the object and transfers handle ownership.

If you do need a copy constructor, then you need a way to share the handle between copies. I would suggest using an auxiallary object to store the handle and a reference count, so you know when to close the handle. I'd also re-examine your assumption that the handle is non-copyable. If the only issue is that you must close it exactly once, then you can work around the problem by using the DuplicateHandle() function.

One final note: I'd say that a cardinal rule in programming is never lie about what a function does.

Answer 7

Perhaps, instead of using a non-standard or wierd copy constructor for an object which is non-copyable, you should consider not copying it. You could use the new shared_ptr machinery or unique_ptr to do ownership handoff instead of worrying about this stuff yourself.

Answer 8

Using a non-const copy constructor is quite OK. I don’t know why you call it “non-standard”. The standard clearly says (§12.8/2) that the copy constructor argument doesn’t have to be const. It just usually is, because modifying the other object is rarely necessary and has its disadvantages (e.g. cannot be used in standard containers).

But I would rather revisit the design: do you really need to copy an object that’s clearly noncopyable? The semantics say otherwise. Alternatively, can’t you copy the underlying handle in a safe way?

• Either just assign it and use shared ownership semantics, perhaps together with a reference counter so you don’t close the handle twice, or too early.
• Or cause the underlying object of the handle to be duplicated.

Answer 9

Both are horrible. If your class has a member variable which is noncopyable, then your class is noncopyable.

If having your class be noncopyable is really unacceptable, one work around is to have a shared pointer to a "state" class/struct to store noncopyable objects (which itself is noncopyable), but your classes can copy the shared pointer around via the standard copy constructor.

Answer 10

As for your edit question: It's not that you're passing a non-const object, it's that you're trying to copy a non-const temporary, which isn't allowed to be passed by non-const reference. You can just declare an actual object and that should correct it.

Fuzz bar() const { Fuzz fuzz; return fuzz; };

Answer 11

The easiest solution is to replace the HANDLE h with a boost::shared_ptr<HANDLE> ph. Now add a private convenience function HANDLE h() { return *ph; }. As a result, the default copy ctor will do the right thing.

Sure, you might have some problems if both objects try to use the handle at the same time. But since your original design left the original object without a handle, this appears to be not the case for you anyway.