Problems with calling factories directly

[sdobbs]

The is a pattern used in various places in the current code which may be causing a problem. In this pattern, factories or plugins will directly access factory objects to use some convenience functions, which can cause a problem.

As an example, inside JEventProcessor_RF_online::Process(), there is the line auto dRFTimeFactory = static_cast<DRFTime_factory*>(locEvent->GetFactory("DRFTime", ""));. Some convenience functions are called from dRFTimeFactory later on (mostly for shifting times by a certain number of beam bunches). However, the functions rely on some CCDB parameters, which are loaded in DRFTime_factory::BeginRun(). It turns out that sometimes this is not being called, which causes the calculation to not work properly.

The question I have is how best to address this problem.

Right now, I can solve it by calling DRFTime_factory::BeginRun() by hand in JEventProcessor_RF_online:: BeginRun(), which is ok since DRFTime_factory::BeginRun() can be called multiple times, nothing bad happens. It's not a great solution though.

The other possible options that I see are:
(1) finding some way to make sure that these factories are properly initialized beforehand - like maybe I need to Get() the DRFTime objects, even if I might not be using them in this processor?
(2) move the convenience functions out into their own objects - some extra code is needed for this, but conceptually is probably cleaner

[rjones30]

Sean, This may be related to a similar problem in one of my plugins that instantiated a private factory in its Init method. It stopped returning objects when I called Get from Process() as soon as I upgraded to JANA2. I found that I had to add a couple of things: a SetApplication followed by an explicit call to the private factory's Init, see below. You may already be doing this? For example, in my plugin Init() method, bc_factory = new DBeamCurrent_factory();

…

bc_factory->SetApplication(app); bc_factory->Init();

-Richard Jones

On Thu, May 8, 2025 at 11:29 AM Sean Dobbs ***@***.***> wrote: *sdobbs* created an issue (JeffersonLab/halld_recon#979) <#979> The is a pattern used in various places in the current code which may be causing a problem. In this pattern, factories or plugins will directly access factory objects to use some convenience functions, which can cause a problem. As an example, inside JEventProcessor_RF_online::Process(), there is the line auto dRFTimeFactory = static_cast<DRFTime_factory*>(locEvent->GetFactory("DRFTime", ""));. Some convenience functions are called from dRFTimeFactory later on (mostly for shifting times by a certain number of beam bunches). However, the functions rely on some CCDB parameters, which are loaded in DRFTime_factory::BeginRun(). It turns out that sometimes this is not being called, which causes the calculation to not work properly. The question I have is how best to address this problem. Right now, I can solve it by calling DRFTime_factory::BeginRun() by hand in JEventProcessor_RF_online:: BeginRun(), which is ok since DRFTime_factory::BeginRun() can be called multiple times, nothing bad happens. It's not a great solution though. The other possible options that I see are: (1) finding some way to make sure that these factories are properly initialized beforehand - like maybe I need to Get() the DRFTime objects, even if I might not be using them in this processor? (2) move the convenience functions out into their own objects - some extra code is needed for this, but conceptually is probably cleaner — Reply to this email directly, view it on GitHub <#979>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3YKWB36OXXSFXRO2KIYI325NZ7FAVCNFSM6AAAAAB4W2AGYWVHI2DSMVQWIX3LMV43ASLTON2WKOZTGA2DSMZWHA4TKNI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[sdobbs]

It's similar, but related, since in my case we are extracting a factory that's supposed to be previously instantiated and managed by JANA.

I could manage a private factory instance, but in that case, I think it would be better to just pull the functions into their own non-factory object.

[nathanwbrei]

I'll always advocate for option (2)! In my mind the whole point of having a framework at all is for dependency injection and inversion of control. In this case you are bypassing the inversion of control and fighting with the (consequent lack of) dependency injection. Also, using framework machinery always comes with a cost in legibility, predictability, and testability -- refactoring these things to not be JFactories will be a relief!

For the sake of getting the code working today without refactoring (which is exactly how these problems accumulate :D ), I'd recommend triggering BeginRun() by calling JFactory::Create(). Create() avoids the ugliness and microscopic performance hit from calling JEvent::Get(), and it is definitely safer than calling BeginRun() directly. If you take a peek inside src/libraries/JANA/JFactory.cc:23, you'll see that there is a fair amount of complexity involved in triggering a factory! Calling BeginRun() is probably fine for your exact use case, but someone might see it and copy it in a different context where it is wildly unsafe. JFactory::Create() is the way to go.

[faustus123]

I'll throw in my 2 cents and say I like the idea of calling JEvent::Get() which returns a pointer to the factory and ensures everything gets called properly. Granted, Nathan says there will be a microscopic (and probably undetectable) performance hit. However, the Get() call will be very familiar to all uses. You will need to cast it though:

std::vector<DRFTime*> rftimes;
auto fac = static_cast<DRFTime_factory*>( locEvent->Get(rftimes) );

[sdobbs]

Thanks for the suggestions! I'll plan to make one of the changes on a temporary basis, and then we can properly refactor this situation.