@aferrero2707 mch rofs records are actually constructed with a bcWidth of 4 by default (

@aphecetche yes, I prefer to have that explicitly set since it is not part of the constructor, just for the sake of clearness and documentation...

@aphecetche I have just realized that the BC width parameter was not actually updated in the setBCWidth(), now it is fixed.

@aphecetche yes, I prefer to have that explicitly set since it is not part of the constructor, just for the sake of clearness and documentation...

well, there's a point to be made about favouring documentation in include file instead of in implementation file, usually I'd expect the information to be readily available in the interface (include) file, not having to dig the implementation.

but ok, putting this point aside, if we go with your changes (btw, good catch about the setBcWith not doing anyway ;-) ), I would remove the mBcWidth{4} data member initialisation in the include file, otherwise we have the constant "4" in two places (and hence risk disconnect and/or confusion).

Why not to have the bcWidth parameter in the constructor? At this point it is just another parameter describing the group of digits. It takes a value of 4 for the ROFs coming out of the decoder, but it can take more or less arbitrary values in ROFs after the time clustering.

I am also realizing that the bcWidth could also be computed a posteriori, given that we know the time-ordered indexes of the first and last digits, and that the digits have their own time stamps in BC units. So the bcWidth information is somewhat redundant...

At the moment the bcWidth parameter seems to be only used in DigitIOV4.cxx.

Why not to have the bcWidth parameter in the constructor? At this point it is just another parameter describing the group of digits. It takes a value of 4 for the ROFs coming out of the decoder, but it can take more or less arbitrary values in ROFs after the time clustering.

Indeed. Seems to be a logical thing to do. But when I introduced the bcWidth I did not want to change the constructor signature, for a reason that escapes me now ... So let's go for it.

I am also realizing that the bcWidth could also be computed a posteriori, given that we know the time-ordered indexes of the first and last digits, and that the digits have their own time stamps in BC units. So the bcWidth information is somewhat redundant...

It's redundant only for digits but ROFRecords are used for other data types as well.

At the moment the bcWidth parameter seems to be only used in DigitIOV4.cxx.

It's also used in the AODProducer to assign a time window to MCH tracks.

@aphecetche ok then I will move the parameter in the constructor and let you know. Thanks!

@aphecetche there are several places where the call to the constructor should be changed when adding one more parameter.
We can either modify the code everywhere, or use a default value for the bcWidth in the constructor. What do you prefer?

@aphecetche there are several places where the call to the constructor should be changed when adding one more parameter.
We can either modify the code everywhere, or use a default value for the bcWidth in the constructor. What do you prefer?

now I guess that may well be the reason why I did not want to change the default constructor at the time 😉 and opted for the bcWidth{4} initialisation, which achieves the same goal with minimal code change...

another option would be to add a 2nd constructor with the bcWidth, for those cases where the bcWidth is non default.

@aphecetche I have added a second constructor with an extra bcWidth parameter. Can you have a look?

Thanks!