billyquith / ponder

C++ reflection library with Lua binding, and JSON and XML serialisation.
http://billyquith.github.io/ponder/
Other
648 stars 95 forks source link

Classes with alternative type id cannot be made properties of other classes #87

Open mickranger opened 7 years ago

mickranger commented 7 years ago

Hi billy,

i really like the library and i'm familiarizing with it. I only struggled when I wanted to make a registered type a property of another registered type, which i consider a quite common case. It took me a while to understand why my code failed.

Currently, ponder::Class::declare() allows to specify an alternative name for the type. However, internally, this is not an alias name but replaces the statically deduced type id (see also #61). The drawback of this decision is that ponder::classByType<>() does not work if the specified name is not identical to the original fully qualified type. For instance, the following snippet would raiseClassNotFound at runtime:

ponder::Class::declare<A>("X");
classByType<A>();

This is not a big deal as long as ponder::class ByType<>() can be avoided because ponder::classByName() can do the trick. However, it is not always possible to circumvent lookup by type. When the ClassBuilder adds a property, it needs to figure out its type. The current implementation deduces the type statically, which results in a very comfortable and safe interface. However, type look-up will fail if the property refers to a registered type which has a name different from the fully qualified type. Thus, calling declare() in the following code would raise ClassNotFound:

struct A {};
struct B { A a; };

PONDER_TYPE(A);
PONDER_TYPE(B);

void declare() {
    ponder::Class::declare<A>("X");
    ponder::Class::declare<B>().property("a", &B::a);
}

That limits the usability of the alternative name quite a bit. Being able to define aliases for type names is invaluabe for serialization, so I suggest separating id and name in Class, making name default to the type, and enable the ClassManager to lookup explicitly by name.

What do you think?

billyquith commented 7 years ago

Sorry, I somehow overlooked this. - Your suggestion makes sense. I plan to overhaul the serialisation aspect at some point (see #43), so this may features in this work. If you are doing serialisation perhaps you have some comments to make on that issue.