Simplify implementation of object factories

[GoogleCodeExporter]

The current "FooFactory" classes are unnecessary. All of them should be 
eliminated, retaining only the "newFoo" functions outlined below:

FalloffFactory:
   Falloff* newFalloff(int type, const vector_fp& coeffs)

KineticsFactory:
   Kinetics* newKinetics(XML_Node& phase, std::vector<ThermoPhase*> th)
   Kinetics* newKinetics(const std::string& model)

SpeciesThermoFactory:
   SpeciesThermo* newSpeciesThermo(int type)
   SpeciesThermo* newSpeciesThermo(const std::string& type)
   SpeciesThermo* newSpeciesThermo(std::vector<XML_Node*>& spNodeList)
   void installThermoForSpecies(...)
   void installVPThermoForSpecies(...)

ThermoFactory:
   ThermoPhase* newThermoPhase(const std::string& model)
   ThermoPhase* newThermoPhase(XML_Node& phase)
   ThermoPhase* newThermoPhase(const std::string& infile, const std::string& id)
   bool importPhase(XML_Node& phase, ThermoPhase* th)

TransportFactory:
   Transport* newTransport(const std::string& model, ThermoPhase*  thermo, int log_level=0)
   Transport* newTransport(ThermoPhase* thermo, int log_level=0)
   LTPspecies* newLTPspecies(...)
   LiquidTranInteraction* newLiquidTranInteraction(...)

ReactorFactory:
    ReactorBase* newReactor(const std::string& reactorType)

Original issue reported on code.google.com by yarmond on 16 Jul 2012 at 9:39

[GoogleCodeExporter]

I strongly disagree with this. By removing the factory classes, you remove the 
capacity for the user to extend the classes and easily handle creation of said 
classes.

Original comment by ptbau...@gmail.com on 10 Sep 2012 at 9:45

[GoogleCodeExporter]

How is the current system extensible by the user? I suppose some (but not all) 
of the newFoo free functions take a FooFactory* which could be a user-created 
derived type, but that only works from the C++ interface, in which case you 
don't really need the factory anyway because you already know you're creating a 
special object type.

If adding proper support for creating user-defined classes using the factories 
is of interest, then we can go that route (although dealing with the static 
initialization order issues involved can be kind of annoying) and keep the 
factory classes. The current implementation just has the complexity without any 
of the benefits, as far as I can tell. 

Original comment by yarmond on 11 Sep 2012 at 2:48

[GoogleCodeExporter]

Take the KineticsFactory for example. There are a couple of newKinetics 
functions that construct Kinetics objects. If I want to use my 
GreatestKineticsSinceSlicedBread class, I could write my own factory that 
derives from KineticsFactory to build it, but still maintain the construction 
options of the library by calling the base class member as appropriate, but not 
having to actually touch the library code.

I'm with you on the static functions and, frankly, I fail to see the utility of 
the build methods for the factory itself and the newKineticsMgr methods (for 
example), instead of just creating a local factory object that builds your guy. 
So, IMHO, ditching the static methods that construct the factory would be OK 
with me, but I view the virtual methods for overloading the construction of the 
"interesting" objects as essential to good extensibility. I have utilized the 
paradigm myself with good results.

One possible improvement is what newKinetics (for example) returns. Right now 
it's a raw pointer, which means someone has to delete it later. In my own 
codes, I like to use shared_ptr, but this is certainly debatable since it 
requires either Boost (making Boost a dependency can be very painful) or a 
C++11 compiler (shared_ptr has been around since gcc 4.3).

Original comment by ptbau...@gmail.com on 11 Sep 2012 at 4:06

[GoogleCodeExporter]

If you want to create a GreatestKineticsSinceSlicedBread, you can just do:

    GreatestKineticsSinceSlicedBread kin;
    importKinetics(phaseXmlNode, vectorOfPhases, &kin);

That seems just as easy as doing:

    MyDerivedFactory factory;
    Kinetics* kin = newKineticsMgr(phaseXmlNode, vectorOfPhases, &factory);

This doesn't require creating a derived factory class, and as a bonus, the new 
Kinetics object can be allocated on the stack. 

What would actually make this factory system extensible would be to keep the 
current factory singletons, and add functions for registering new object types, 
so you could do something more like this:

    Kinetics* kin = newKineticsMgr("GreatestKineticsSinceSlicedBread");

This way, you could actually create user-defined object types from generic 
code. For instance, this would actually allow you to use the new class from the 
Python or Matlab interfaces without any further modifications. 

I think this approach is much more general than creating derived factory 
classes, and hopefully simpler for end-users to work with. Getting the 
registration to work properly as part of static initialization can be a little 
tricky, but might be worth the effort if we want to provide better support for 
user-created types.

Original comment by yarmond on 11 Sep 2012 at 5:56

[GoogleCodeExporter]

Regarding the first part, yes, it seems just as easy, but it's much less 
flexible. You now have to recompile each time you want to switch a kinetics 
object whereas with the other, you can still bring this information in from an 
input file.

Regarding the second part, this could be interesting and I could get on board 
with it. It would need to be general enough that information is brought in from 
the input file. I disagree that it's "much more general" (for example, you're 
all kinetics objects are going to have to have the same constructor arguments 
as well as losing separation of object creation from the class). I do agree it 
will be simpler (by only a few lines of code) for the user the enable a new 
derived object. I'm unfamiliar with how the Python or Matlab interfaces work, 
but if such paradigm enables ease of use with these interfaces and the other 
does not, then this is likely better for the library as a whole.

Original comment by ptbau...@gmail.com on 13 Sep 2012 at 2:34

[GoogleCodeExporter]

Having thought about this some more, I think that we can simplify things a bit 
by removing the optional "factory" argument from the free "newFoo" functions. 
These functions should always use the default factory. If you need to use a 
custom factory, you must already have an instance of it, so there's no reason 
not to just use its methods directly.

Here's a rough example of what I was thinking for the making additional types 
registrable:

    template <class T>
    class Factory
    {
    public:
        typedef T* (*constructor)();

        static T* create(const std::string& name) {
            if (types().count(name)) {
                return types()[name]();
            } else {
                return 0;
            }
        }

        static int addType(const std::string& name, constructor f) {
            std::cout << "registering " << name << std::endl;
            types()[name] = f;
            return 0;
        }

    private:
        static std::map<std::string, constructor>& types() {
            if (!m_types) {
                m_types = new std::map<std::string, constructor>;
            }
            return *m_types;
        }

        static std::map<std::string, constructor>* m_types;
    };

Then, if you have a set of classes:

    class Foo { ... }
    class Bar : public Foo { ... }
    class Baz : public Foo { ... }

Introduce a factory function for this base type:

    template<> std::map<std::string, Factory<Foo>::constructor>* Factory<Foo>::m_types = 0;
    Foo* newFoo(const std::string& name) {
        return Factory<Foo>::create(name);
    }

And then register the derived types. The most direct (and flexible way):

    Foo* make_bar() {
        return new Bar();
    }
    int dummy1 = Factory<Foo>::addType("Bar", make_bar);

    Foo* make_baz() {
        return new Baz();
    }
    int dummy2 = Factory<Foo>::addType("Baz", make_baz);

You can use macros or templates to get rid of most of this boilerplate if you 
have a common signature for the constructor, but it's not a requirement of the 
registry that all the objects have the same constructor.

The downside of this (and every other method I've seen for a factory registry) 
is that it doesn't work when the registration functions are in a static library 
because the symbols for the dummy variables get dropped at link time. We could 
have all of built-in types added to the factory as part of the factory's 
initialization, and then types added by the user would be included if they 
either put their code in a shared library or have an initialization method make 
the call to addType().

Original comment by yarmond on 7 May 2013 at 12:38