billyquith / ponder

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

PONDER_AUTO_TYPE does not appear to work with classByName #45

Closed iwbnwif closed 8 years ago

iwbnwif commented 8 years ago

If a class, whose registration is achieved by PONDER_AUTO_TYPE, is not registered when it is referenced using classByName, then a ClassNotFound exception is thrown.

billyquith commented 8 years ago

Are you sure the name is correct? byName and byId use the same retrieval call. Generally I'd advise against giving classes a name and just use the byId methods.

iwbnwif commented 8 years ago

Here is a code snippet that shows the problem. It is based on the Person class given in the example.

void RegisterPerson()
{
    // Bind our Person class to Ponder
    ponder::Class::declare<Person>("Person")
        .constructor<std::string>()
        .property("name", &Person::name)
        .property("title", &Person::title)
        .property("age", &Person::age, &Person::setAge)
        .function("speak", &Person::speak);
}
PONDER_AUTO_TYPE(Person, &RegisterPerson);

...

    // Retrieve the metaclass by its index (cannot use with PONDER_AUTO_TYPE)
    // const ponder::Class& metaclassByIndex = ponder::classByIndex(0);

    // Retrieve the metaclass by its type (works)
    // const ponder::Class& metaclassByType = ponder::classByType<Person>();
    // std::cout << "Registered " << metaclassByType.name() << std::endl;

    // Retrieve the metaclass by an object (works)
    // Person myPerson("Dan");
    // const ponder::Class& metaclassByObject = ponder::classByObject(myPerson);
    // std::cout << "Registered " << metaclassByObject.name() << std::endl;

    // Retrieve the metaclass by its name (fails - Error: the metaclass Person couldn't be found)
    const ponder::Class& metaclassByName = ponder::classByName("Person");
    std::cout << "Registered " << metaclassByName.name() << std::endl;

As per the comments, classByType and classByObject both work perfectly, but classByName does not work unless the class is already registered.

billyquith commented 8 years ago

Yes, I've verified that this happens. The problem is that types are automatically registered via static type lookup by type. When you use a string lookup the static type system is not used.

This could be fixed by refactoring the automated registration, but a simpler solution is to just use non-automated PONDER_TYPE(...) registration where you intend to access by name. This is also more efficient if you do a lot of lookups since, currently, the automated type lookup also checks the type is registered on every call.

If you have a data driven system, where everything is driven by strings I'd advise registering everything up-front.

iwbnwif commented 8 years ago

Thanks for confirming.

but a simpler solution is to just use non-automated PONDER_TYPE(...) registration where you intend to access by name

The reason for not doing this is because I have multiple 'generic views' that access class instances. Registration of classes depends on how the user opens the views and may cause multiple registration attempts. I am handling this at the moment by ignoring ClassAlreadyCreated exceptions in the registration function.

If you have a data driven system, where everything is driven by strings I'd advise registering everything up-front.

Thanks for the tip. What I have are singleton 'service' classes that sit between the views and the database so I can register the relevant classes there, which I now think is better on many levels :)

I will leave the issue open in case you think it is worth refactoring or mentioning the limitation in the documentation. Otherwise, please feel free to close it.

billyquith commented 8 years ago

I've added a note to the docs, which I've updated. Also includes macros docs and search! 😄

iwbnwif commented 8 years ago

It's great to have the search function enabled.