While there is nothing stopping a developer from registering recipes
throughout the simulation, this feature is primarily provided to facilitate
simulation setup.  Agents should acquire pointers to externally loaded
recipes and then just keep them internally - I don't anticipate performance
aware developers calling these methods more than once for needed recipes at
the beginning of a simulation.

You mention iteration, but I don't think we should cater to agents that
aren't sure of exact recipes they are looking for.  That could make
simulation results much less deterministic.  If an agent doesn't know about
a specific recipe (or recipes) to retrieve, then part of provenance
requires trusting the agent to document which recipe it actually finds and
uses.  If it expects a recipe that isn't there, abort.

The methods also add some convenience.  I am not universally against public
members.  However, there are times when they are useful, it is currently
the convention in the cyclus codebase to keep them private, and the style
guide also advises against public members.  The style guide says "...and
provide access to them through accessor functions *as needed" *(emphasis
added).


On Fri, Sep 6, 2013 at 10:15 AM, Anthony Scopatz
<notifications@github.com>wrote:

> In src/Core/context.cc:
>
> > +};
> > +
> > +void Context::RegisterProto(std::
> string name, Prototype* p) {
> > +  protos_[name] = p;
> > +}
> > +
> > +void Context::RegisterRecipe(std::string name, Composition::Ptr c) {
> > +  recipes_[name] = c;
> > +};
> > +
> > +Composition::Ptr Context::GetRecipe(std::string name) {
> > +  if (recipes_.count(name) == 0) {
> > +    throw KeyError("Invalid recipe name " + name);
> > +  }
> > +  return recipes_[name];
> > +};
>
> My shallow response is that it also tells you to just have an accessor
> called recipe() which should be inlined [1] =). (It also advises you not
> to overload operators which is ridiculous.) [1]
>
> I understand that map's behavior is weird and undesirable. However, you
> are cutting off a lot of functionality -- ie the map<T, U> API to prevent
> another developer from accidentally not writing robust code. It is 1 - 3
> lines for the developer to write the same kind of safety guard that you
> have here. Right now the developer has no mechanism to iterating over the
> existing recipes. Maybe I missed it. I'd rather not redefine a healthy
> portion of map just to keep a member variable private.
>
> It just seems to me that you could have more features with less code by
> having these maps be public. If you wanted to keep the Get/Add functions
> around too as the safe mechanism, that would be fine. Though I think Add/
> only serves to slow the program down in its present state.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/cyclus/cyclus/pull/568/files#r6210306>
> .
>