[Discussion]: Parsing - Object Constructors or Factories
Opened this issue · 1 comments
Wanted to capture some thoughts for discussion while I was thinking about it. Aimed mainly @roystgnr but anyone who's reading can feel free to chime in.
Going through and updating the way we build objects based on FactoryAbstract got me thinking more about how we build our objects, in particular with having to parse everything using GetPot. Originally (a long time ago, etc.), I had been thinking have every object take GetPot and worry about it's own things to avoid the catch-everything-in-input style that we had in FIN-S.
Overtime, the parsing has gotten more complicated and connected since we want to minimize duplication in the input file while at the same time sharing information between modules, e.g. only one Variables section in the input file while, at the same time, all the Physics classes should be able to understand the variables.
I've been wondering if maybe the parsing work should be done in the Factory objects. That is, the objects we're building (Physics, QoI, Solver, etc.) should be "pure" in the sense that they get handed everything they need to start working (perhaps needing an init() call before computation, but otherwise no parsing or anything): pass by value for constants, pass by (const) reference for things the object doesn't own (e.g. variables are maintained in the VariableWarehouse since lots of things use the variables), shared_ptr for things that have shared ownership and could potentially be NULL (I can't think of anything off-hand that fits this bill), and unique_ptr for things it will own.
Obviously, such purity cleans up the guts of our "core" objects and moves the parsing complexity to the factory, which is both good and bad. The good is easier testing and understanding of the core objects (both for users and developers) and we separate all the complicated parsing semantics away from the functionality of the core object. It would also move the parsing out of the parameter registration: the Physics constructor, for example, could just directly register the objects that it's handed and not worry about the parsing or error checking issues.
The bad is that each leaf object may have slightly different things it needs so then we end up with nearly 1 factory for every object. That does sound bad, but I wonder if maybe that can also be mitigated to some degree because now things like multiple inheritance makes sense for the factories. For example, instead of VariableParsing being static-all-the-things, it becomes non-static and then PhysicsFactory and QoIFactory now also inherit from VariableParsing because they are variable parsers (among other things). So maybe we can gain a lot more contained reuse in the factories to help offset the (likely) bloat in the number of leaf classes for the factories.
I like the sound of trying to contain the construction of things more and moving the parsing away from the core objects, but I wonder if there's other drawbacks I'm not thinking of. Wanted to get some reactions to this. Was thinking about using new QoIFactory as an experiment for this to see how it worked out.
I think we're already past optimum on the factory patterns. Opening up a random .C file in GRINS reminds more more of the final example than the original example in https://taskinoor.wordpress.com/2011/09/21/the-abuse-of-design-patterns-in-writing-a-hello-world-program/
That said, I'm not against more modularity. I just don't think the Factory objects are the place to put it. If each WhateverPhysics class got a WhateverPhysicsParser friend class and all GetPot interaction moved into the latter, it would be a nice conceptual separation but wouldn't increase the line count much.