aspect
aspect copied to clipboard
Initialization order is error-prone.
In working on #4889, I realized something that had previously occurred to me: When we create the various plugin systems in the constructor of Simulator
, we create some of the variables in the member variable initializer list and others in the body of the constructor. For the latter, we generally follow logic such as
for (const auto &p : parameters.prescribed_traction_boundary_indicators)
{
BoundaryTraction::Interface<dim> *bv
= BoundaryTraction::create_boundary_traction<dim>
(p.second.second);
boundary_traction[p.first].reset (bv);
if (SimulatorAccess<dim> *sim = dynamic_cast<SimulatorAccess<dim>*>(bv))
sim->initialize_simulator(*this);
bv->parse_parameters (prm);
bv->initialize ();
}
This is awkward (and cost me ~2 hours of debugging in #4891) because during initialization of some objects, we are accessing other objects via the SimulatorAccess
class. A better design ensures that we first create all objects in a first phase, and then initialize all in a second phase. I believe that we should split that second phase into calling parse_parameters()
for all objects first, and then call initialize()
later -- so, three phases.
Makes sense to me.
I think this is reasonable. One thing you need to be careful with is that the order of the plugin systems remains exactly the same. The current order is the result of a lot of reshuffling until all dependencies are satisfied. I hope none of the plugins require initialized information inside parse_parameters
(but if they do the tests hopefully catch it, and that should be fixed anyway).