Jörg, new to the project as I am, I do like the general idea. After a few glances at the code however I found a few issues.
The code could use some more explanation as to which class does what, better do that now before it grows. A common use scenario showing what users of the code will do and what is internal affairs between a business object and it's factory would help, as it's not immediatelly clear. If I understand correctly, wml_object::build_object takes a pointer to an already initialized buiness object, and silently assumes it is actually the proper derived class by casting in child classes. This pretty much kills compile-time type safety, which I don't like very much, and which might lead to some very subtle bugs. At the very least, use dynamic_cast<> instead of a c-style cast so it will crash when the cast is impossible instead of having undefined behaviour. Downcasting, however, sometimes effect_factory::create_effect does a new(), but then returns a reference instead of a pointer o what's been allocated. This might lead to a memory leak. Shouldn't the classes that build business objects (wml_object and derived) be singletons? I don't like the many-stage business_object::build, in fact this function pretty much combines several flaws. It uses new/delete, which is asking for trouble and also slow. It creates a possibly large object only to delete it a moment later. Also, why the initialize()? I think an object should be ready to use after construction without any init stuff if possible. Seems to me that code such as "unit_trait trait; trait.bulid(cfg);" will be used often, and shouldn't incur this much overhead. I think the code could use some singletons and a good bit of the factory pattern. There is something in this direction in effect_factory, but I think it should do all the work and create the proper unit_effect from a config. There might be stuff I overlooked or stuff that warrants what I've criticised, feel free to correct me. What were you planning on using templates for? They are a compile-time mechanism in many ways orthogonal to inheritance, and shouldn't really prevent you from doing anything. Cheers, -- Tomek "ilor" Śniatowski _______________________________________________ Wesnoth-dev mailing list [email protected] https://mail.gna.org/listinfo/wesnoth-dev
