Static Deserialization Factory in C++ Interface

Question

Is it bad practice to have the static deserialization method in the interface class as below?

class IBase
{
  ~IBase();
  virtual void Foo()=0;

  virtual const char* Serialize()=0;
  static std::unique_ptr<IBase> Deserialize(const char* data); // <-- This
};

IBase will travel across DLL boundaries, so an effective method of serialization is for each object to serialize itself, hence the IBase::Serialize function.

Then when we have raw data and we want to deserialize back to an object, it seems logical to call IBase::Deserialize, so that the Serialize and Deserialize functions are closely coupled.

boost::serialization does some clever automatic stuff so that the Deserialize factory function just looks like data >> pIBase;, so there's no manually updating the factory function for new derived types.

The alternative as I see it is to create an IBaseDeserializer class to hold the Deserialize function:

class IBase
{
  ~IBase();
  virtual void Foo()=0;

  virtual const char* Serialize()=0;
};

class IBaseDeserializer
{
  static std::unique_ptr<IBase> Deserialize(const char* data);
};

Most of the similar questions on SO claim that the first method should be avoided because it violates Single Responsibility Principle, having the base interface also be a factory. However, I feel as though this is a special case in the sense that it's a corresponding function to the base interface's Serialize function, which just happens to be a factory. However, it doesn't make a lot of sense for a derived object to have a Deserialize function, or for an interface to have an implementation, albeit static.

After writing this I'm leaning more towards the second method, but I still see value in the first. What's best practice here? What other pro's and con's are there, or am I way off base (no pun intended) altogether and should be doing something else?

Answer 1

You're gonna run into a helluva lot more problems passing those std::shared_ptr's across DLL boundaries unless you've used the exact same compiler and standard library for all your DLL's and their respective clients or you get really lucky. (And same goes for std::unique_ptrs. It's the same issue you've already identified with std::string).

Your use of a static deserializer isn't good or bad practice, it's just an arbitrary design choice. Just stay consistent, that's the important part.

But you really ought to find a way to return IBase *'s and keep any shared pointer stuff isolated to the client-side, or consider rolling your own COM-style reference counted approach, or just straight up use COM (dealing with shared objects across DLL boundaries is one of the problems COM was designed to solve).

And it's not just return types. Even e.g. std::shared_ptr (or std::string for that matter) member variables in the DLL headers will be problematic, as their size / member layouts may be different across compilers, you'd have to hide anything like that behind the so-called pimpl pattern. Same deal with throwing std::exceptions across DLL boundaries.

---

Addressing your comment with a contrived example (I'm keeping this really simple, I hope it's representative even though it's not using any template stuff, the templatedness isn't really the issue):

Consider some stuff exported from a DLL:

// In a header you have no control over, e.g. a standard library header:
struct Example {
    int x;
    int y;
};

// Exported from your DLL:
class IBase {
public:
    static Example constructExample (int x, int y);
    static void incrementX (Example *ex);
};

// And its implementation:
Example IBase::constructExample (int x, int y) {
    Example ex;
    ex.x = x;
    ex.y = y;
    return ex;
}

void IBase::incrementX (Example *ex) {
    ex.x ++;
}

Now, you commented "... won't cross DLL boundaries as it's a static function - i.e. compiled into whatever DLL that's calling them". But it will. For example, in a client using that DLL:

// In a SLIGHTLY DIFFERENT VERSION of that header you have no control over, 
// e.g. a standard library header:
struct Example {
    int y; // note x and y are swapped from the version of this header
    int x; // that the DLL was compiled with.
};

// In your client code somewhere:
Example ex = IBase::constructExample(1, 2);
assert(ex.x == 1); // <- will fail, our broken ex.x is actually 2.
assert(ex.y == 2); // <- will fail, our broken ex.y is actually 1.

// And also:
Example ex;
ex.x = 0;
ex.y = 0;
IBase::incrementX(&ex);    
assert(ex.x == 1); // <- will fail, because our broken ex.y was incremented
assert(ex.y == 0); // <- will fail, because our broken ex.y was incremented

As you can see, the fact that it is static does not make a difference here.

Answer 2

The common practice is:

class IBase
{
  ~IBase();
  virtual void Foo()=0;

  // this means "Dump your content into output"
  virtual obj_ostream& Serialize(obj_ostream& )=0;

  // This means "fill you content anew from input (destroy your old one if necesary)"
  virtual obj_istream& Deserialize(obj_istream& )=0;

  // or you may prefer the >>, << operators
};

Note the use of obj_stream classes - you'll need to derive your own even if you inherit from or wrap-around^** basic iostreams. This way you don't trip over l/r-shift operators others may want to write for the std::iostream.

Also, the use of streams answers the question "who is going to free the buffer transporting your data"

---

Now, addressing the "which of the two solution to choose" question. It's absolutely not important, as both of the solutions are absolutely equivalent and equivalent with this one:

class IBase
{
  ~IBase();
  virtual void Foo()=0;

  virtual const char* Serialize()=0;
};

std::unique_ptr<IBase> Deserialize(const char* data); // <-- This

The only difference between them is the context in which the "factory" function is declared: in your first, is a static method of IBase, your second... yeap... still a single (static) function declared by the IBaseDeserializer class, the above yet another declaration place: the same namespace your IBase class is.

Look... it's a static method, you won't be able to override it in any derived classes.
It will have a single non-polymorphic identity and functionality no matter where you declare it (quibbling over the differences is nitpicking in my opinion).
At the best, placing it outside any class saves you the pain of typing the typing the class scope when you call it.

---

^** I find it easier to use (composition) a wrapped stream rather than derive from iostreams (actually basic_ostream/basic_istream)