I implemented a class to handle some external functions (e.g of another DLL). This functions gives me an integer I can use as a handle. Here is the important part of my code:
MyClass
{
public:
MyClass() {
handle = getHandlefromExternalFunction();
}
~MyClass {
if(handle>0)
freeHandleFromExternalFunction(handle);
}
MyClass& operator=(MyClass& other) {
freeHandleFromExternalFunction(handle);
handle = other.handle
other.handle = 0; //Is this a bad idea?
}
private:
int handle;
}
In my main function I have an object of myClass. At some point I am using the assignement operator to change the values of the object:
MyClass object;
//some code
object = MyClass();
After assignement the object created by MyClass() is immediatly destroyed since it gets out of scope. But I don't want freeHandleFromExternalFunction() to be called on that handle, since I am using it in the assigned object. Therefor I change the value of the assigned object in the assignement operator handle = 0. My question is: Is this a bad idea? Has anybody a better solution for my problem?
Yes it's a bad idea. You don't normally expect the right-hand side of an assignment to be modified.
If you want to move ownership then use the "move" assignment operator together with std::move:
MyClass& operator=(MyClass&& other) { ... }
// ...
MyClass a = ...;
MyClass b;
b = std::move(a);
If you only want movement like this (where the can be only one owner of the contained resource), then I also suggest you mark the copy-constructor and copy-assignment operators as deleted:
MyClass& operator=(MyClass const&) = delete;
MyClass(MyClass const&) = delete;
And following the rule of five don't forget the move-constructor and destructor:
~MyClass() { ... }
MyClass(MyClass&& other) { ... }
class MyClass {
public:
//....
MyClass& operator=(MyClass& other)
Non-const other in an assignment is a bad idea, and can easily surprise a programmer. Most programmers don't expect the right hand side of an assignment to be mutated.
Also this won't compile:
MyClass obj;
obj = MyClass(); // error
For this to compile you must use move semantics, which is probably what you meant from the start:
class MyClass2 {
public:
//....
MyClass2& operator=(const MyClass2& other) = delete;
MyClass2& operator=(MyClass2&& other)
The && means that the other may be emptied in the process:
MyClass2 obj, obj2, obj3;
obj = MyClass2(); // ok. Will be moved
obj2 =std::move(obj); // ok
obj3 = obj2; // error, not an rvalue rference (not MyClass2 &&)
Make sure no two objects hold the same handle. Make sure copy and assignment are deleted, and move assignment and move constructor nullify the handle of the right hand side.
Your object should own the handle. The handle should have only one owner.
