Has Pointer Member but does not override..

[triumphost]

"warning: 'class Debug' has pointer data members but does not override 'Debug(const Debug&)' or 'operator=(const Debug&)' [-Weffc++]"

The only pointer member I see here is the HANDLE hTHread. Why should I override my constructors or add a move/copy to this class as the compiler suggests?

class Debug
{
    private:
        HANDLE hThread;
        bool ConsoleActive;
        friend DWORD ThreadProc(LPVOID lpParameter);
        friend long __stdcall WindowProcedure(HWND window, uint32_t msg, WPARAM wp, LPARAM lp);

    public:
        Debug();
        explicit Debug(bool CreateDebugConsole = false, bool CreateDebugWindow = false);
        //Other constructs and destructor..
};

It says the same for:

class Brushes
{
    private:
        HDC     hDC;
        HBRUSH  NewBrush;
        HBRUSH  OldBrush;

    public:
        Brushes(HDC hdc, COLORREF colour);
        //Destructor and other constructors here..
};

I read this tutorial: http://pages.cs.wisc.edu/~hasti/cs368/CppTutorial/NOTES/CLASSES-PTRS.html
It doesn't use the copy swap idiom or something like that but it explained to me the basics. I just don't see a reason to use it for HBRUSH/HPEN or Handles :S?

[deceptikon]

Why should I override my constructors or add a move/copy to this class as the compiler suggests?

As the compiler suggests? I assume you're getting a warning to that effect, what does it say?

[NathanOliver]

You want to write your own copy constructor for any class that contains pointers. The reason for this is that the default one the compiler makes only performs a shallow copy. This means that the copy will be exactly the same as the original. With pointers this is bad because if the copy gets destroyed which happens a lot with copies the pointer gets deleted. Once this happens the pointer in your original class can no longer be used safely and you might even get an exception when you delete the original class since you would call delete on the pointer again. IMHO I write my own copy constructor unless the class I am writing contains only POD types or classes that take care of themselves like vectors or strings.

[triumphost]

@Nathan. The reason I was asking was because it's a handle. And it's a handle to a thread. Copying a handle will just make it point to the same data. I understand the move constructor will just steal the pointer to the thread aka the handle but copying? I do not understand the copy part of that.

If I'm to copy a handle, both will still point to the same place because I cannot copy a thread or a Pen/Brush. When the old object gets destroyed, the thread will get destroyed and the handle to the thread in the new object will point no where.

Only thing I can think of is to copy the handle and set the old one to 0. If I set the old one to 0, the Old object will have an invalid handle. Copy con is supposed to copy not steal :S. In the case of Brushes I'm confused though because I'd copy the handle to the brush and then what? I can't just set it to 0. I have to select the object back no? The destructor selects it back..

I'm just not sure how to deal with handles.

[NathanOliver]

according to http://msdn.microsoft.com/en-us/library/windows/desktop/aa383751%28v=vs.85%29.aspx the HANDLE type is a void *. If that is the case than in your copy constructor you should creat a new HANDLE and than asign the value of the copy from HANDLE to the copy to HANDLE. I cant be certian as I havent done any windows api programing though.

[triumphost]

But that's not possible :S I cannot copy a void* with no size. I can't do a memcpy on it because it's a thread I can't do std::copy because the thread has no iterator and no size either. Can't do sizeof(hThread) because it's just a void pointer.

For the Brushes they are also defined as void* so how do I copy that? I understand if I knew the size or if it pointed as something reasonable but in this case it's very difficult. Maybe I can dereference it and attempt to copy but I know almost 100% that it won't work.

I'm not sure that I can copy a thread or an hBrush. It seems closely related to copying the handle of a file and closing one but not the other. Then the other will just dangle there.

Yet the compiler complains about me not providing a copy con or move con. I made one to get it to shut up but the one I made is a shallow copy. Move con is fine. Copy con definitely isn't. When the handle in one object is destroyed, the handle in it's copy is as well. I'm not sure whether to ignore the compiler or not because this is very silly but I always learned that I should do the copy and move when I have pointers.

EDIT: I tried putting it into a Smart Container. Uniqueptr to be exact but it doesn't help. Neither does shared. It still complains.

[mike_2000_17]

The HANDLE is what one calls an "opaque pointer". This just means that it is essentially a pointer but you have no idea what it points to, and you're not supposed to know (it's "opaque").

If you library from which you got the opaque pointer does not have a method to copy (or duplicate) the resource that it points to, then you cannot copy it, and you shouldn't attempt to.

If you should copy something, then you must disable copying. This can be done in a number of ways, depending on what you've got to work with. The basic technique is to declare the copy-construct as a private member function and not provide a definition for it, so that any attempts to call the copy-constructor is going to result in a compilation error:

class Foo {
  private:
    Foo(const Foo&);  // private copy-con, no definition (declaration-only).
    Foo& operator=(const Foo&);  // private copy-assign, no definition either.
  public:
    //... the rest of the class.
};

Another possibility is to inherit from boost::noncopyable which is basically just a simple class like the Foo class above, that declares private copy-con and copy-assign. Finally, if you have a C++11 compiler (as I assume from your mentions of move-constructors), you can use the explicit deletion of the constructor, as follows:

class Foo {
  public:
    Foo(const Foo&) = delete;  // non-copyable
    Foo& operator=(const Foo&) = delete;  // non-copy-assignable
};

The effect is the same as with a private, undefined copy-con and copy-assign, but the error messages will be clearer.

If you want move-semantics for you class, you should implement them (move-construct and move-assign), otherwise, explicitly delete them as well.

Once you have such a non-copyable class, if you need multiple parts of the code to access that object, then use a smart-pointer, like std::shared_ptr<Foo>.

[triumphost]

Ohhhh I see! That never occured to me that such a thing can be done. I have never seen such code before.

I created the class myself and it was a handle returned from CreateThread but there's no way to tell what data it actually holds. All I know is that CreateThread gave me the handle and the compiler kept complaining about it being a pointer member and the move and copy con.

I'll look into your suggestions then I'll implement it but I have to learn it first. Thank you lots! I'll mark it solved in a sec. Hopefully it works.

EDIT: With that implementation, is it still safe to supply a move constructor? Should I supply one or is it just not necessary?

[mike_2000_17]

That never occured to me that such a thing can be done. I have never seen such code before.

The non-copyable idiom is a classic technique in C++. It is not used as much as it should, but it is a nice technique, it is one of those nice and simple examples of how you can get the compiler to help you to write correct code (by forcing it to produce compilation errors when the code is not correct).

With that implementation, is it still safe to supply a move constructor?

Yes. The move constructor generally makes sense for this case. You might not really need it though, but it doesn't hurt to have one. If you want to get more or less the same behavior as move-semantics, but remain legal C++03 (previous standard), then you need to define a default constructor (null handle) and a swap function.

Towards the end of my tutorial on RAII in C++11, there is a non-copyable class example (a home-brewed array container).

Should I supply one or is it just not necessary?

It is not necessary, unless you need to move the object around, but it doesn't hurt to have it anyways (and you never know, you might want to have a container of such objects (like a thread-pool), which is possible even if the class only has move-semantics, with C++11 enabled STL containers).