Possible to have the generators know their channels in their constructors?

[AndrewEdmonds11]

As mentioned in pull request #211, I have a question about the TAP generators. Here's what I wrote:

Also, I wanted to set all the parameter values and create the algorithms in the generator's constructor rather than in the ProcessPulses method since (if I've understood correctly) there is one instance of a generator per channel. However, the generator's channel is not set by the time we get to the constructor. We saw this problem previously (this commit 68b6cad447b8e3d26d6bcdc9e24743d182b78e00 shows when we overloaded the SetChannel method to get the PCF in the FirstCompleteAPGenerator) but I'm wondering if it's simple and worth having it so that the generator's channel is set when it's constructed? (Again, I'll open a separate issue for this)

@benkrikler, I guess you're the best person to answer.

[benkrikler]

The reason I'm reluctant to change the generator's constructor is because by keeping them all the same as the modules' constructors I can reuse a lot of code.

The code from MakeAnalysedPulses::BeforeFirstEntry responsible for instantiating the generators is:

203     // Get the requested generator
204     TVAnalysedPulseGenerator* generator=
205         TAPGeneratorFactory::Instance()->createModule(generatorType,opts);
206     if(!generator) return false;
207     generator->SetChannel(detector);

There are 2 ways I could improve this, and in the end it might be worth doing both:

1. Add the channel's name as an option that get's passed in:

   203     // Get the requested generator
   204     opts->SetOption("channel",detector);
   205     TVAnalysedPulseGenerator* generator=
   206         TAPGeneratorFactory::Instance()->createModule(generatorType,opts);

2. We add an Initialise method to the generators equivalent to BeforeFirstEntry for modules. I would call it immediately after the constructor. This is a bit more convenient if we want restructure things, though it may not be used very often at this point.

[AndrewEdmonds11]

Cool. It's nice that you have ideas to implement.

Do you think it is a massive slow-down having the retrieval of parameters and creation of the Algorithms in the ProcessPulses function though? From a conceptual point of view it would be nice to move these things into the constructor but if it's not worth it then there's no point in making any extra work for ourselves?

[benkrikler]

Do you think it is a massive slow-down having the retrieval of parameters and creation of the Algorithms in the ProcessPulses function though?

As long as it's a one off operation that's only called on the first loop the effect of keeping this in the ProcessPulses method would be small and potentially zero if the compiler optimises this (though I really don't know enough of that to be sure).

I can very easily add the channel name to the options passed in though. Then you can work in the constructor as you'd like. I'll call the option channel_name which I assume is not being used as the option to any generator at the moment.

We can just keep in mind the option of an Init method for the future, though I won't add it now.

[AndrewEdmonds11]

Cool. Thanks, Ben. That makes things easier.

[benkrikler]

This is done on develop.