I'm building a DLL and in my public headers I have this: (definitions are in .cpp but for clarity I show them in .hpp here)
ObjectTag.hpp:
class API_DLL ObjectTag {
public:
ObjectTag() : mUUID(UIDGenerator::Generate()) {
}
uuid getUUID() {
return mUUID;
}
private:
uuid mUUID;
};
Texture.hpp:
class API_DLL Texture : public ObjectTag {
public:
Texture() : ObjectTag(){
}
};
I have a Scene editor that creates Texture objects, and saves them in a file with their uuid. Now I want to load those Texture objects read from the file inside the library and set their previously generated and saved UUID.
My first attempt was to make this simple function void setUUID(uuid id):
class API_DLL ObjectTag {
public:
ObjectTag() : mUUID(UIDGenerator::Generate()) {
}
uuid getUUID() const {
return mUUID;
}
void setUUID(uuid id) {
mUUID=id;
}
private:
uuid mUUID;
};
But this will allow the users of the library to modify the UUID variable. How would you design this in a way that prevents doing that from the users of the library?
2 Answers 2
How would you design this in a way that prevents doing that from the users of the library?
Separate Texture interface from implementation:
class API_DLL ObjectTag
{
uuid mUUID;
public:
uuid getUUID()
{
return mUUID;
}
};
class API_DLL Texture: public ObjectTag // Texture interface visible in client code
{
public:
virtual ~Texture() = 0;
virtual void DoTextureThings() = 0;
};
template<typename T>
class EditableObjectTag: public T // EditableObjectTag not exposed to client code
// defines save functionality
{
public:
void setUUID(uuid id) { mUUID = id; }
};
class YourTexture: public EditableObjectTag<Texture> // YourTexture not exposed
// to client code
{
void DoTextureThings() override { ... }
};
Your load function:
API_DLL std::vector<std::unique_ptr<Texture>> LoadTextures()
{
std::vector<std::unique_ptr<Texture>> result;
result.emplace_back(new YourTexture{ bla, bla, bla });
return result;
}
Client code can now work with textures, without caring that a Texture is an abstract class, and the Texture interface doesn't mention anything about setting values into the object:
void ClientCode()
{
auto textures = LoadTextures();
textures[0]->DoTextureThings();
// textures[0]->setUUID(someUUID); -> fails to compile
}
Note: the EditableObjectTag class is an aspect-based design: it adds an editable interface on it's template argument.
If you want to serialize and deserialize Texture objects, then you should have constructors, methods or interfaces explicitly designed for this purpose. Serialization is one of the few use cases that's special enough to deserve it.
Since this is C++, you have many options. Off the top of my head:
- Give the
Texture
classserialize()
anddeserialize()
methods. Simplicity is a virtue. - Make a separate
TextureSerializer
class that's afriend
of theTexture
class so that you can makesetUUID()
private. This approach is often a good idea if you expect serialization to be complicated or have non-trivial dependencies or theTexture
class already has a ton of stuff in it. - Create a
Serializable
class with virtualserialize()
anddeserialize()
methods and no state, a.k.a. an interface, and makeTexture
implement that interface. If you expect other programmers that might write their own serializable classes that your program has to deal with, this might be the safest and most generic approach. - Create a
SerializableTaggedObject
base class with virtualserialize()
anddeserialize()
methods and theuuid
member and uuid-generating constructor you currently have inObjectTag
. If you have several classes similar toTexture
, which both need to be serializable and have uuids, this might be the best choice.
All of these options will protect you against non-malicious users who simply didn't read whatever comments you wrote on top of setUUID()
. As you can see from my additional remarks, which one you actually pick should depend on a lot of other factors, but any of them would be a huge improvement over exposing a public setter for a field that's supposed to be immutable.
I'm deliberately skipping a few implementation questions such as whether any of those methods should be static. And I'm assuming you aren't trying to protect against the malicious users who would gladly call Texture.deserialize("{ \"uuid\": \"bwahahahaha\" }")
instead of setUUID()
, but merely prevent users from mistakenly triggering undefined behavior in your code.
P.S. Since you specifically mentioned DLLs, I feel I should also mention the pimpl idiom, since that hides implementation details in a way that often improves binary compatibility, which tends to be a particularly strong concern for DLLs. Of course you still have to put a serialization mechanism somewhere on the real implementation, so this would be in addition to one of the options I described above.
setUUID
is only for internal use and should not be called by user code?