JeffersonLab / JANA2

Multi-threaded HENP Event Reconstruction
https://jeffersonlab.github.io/JANA2/
Other
6 stars 9 forks source link

Strong types for event number and run number? #267

Closed nathanwbrei closed 3 days ago

nathanwbrei commented 7 months ago

In EICrecon's JOmnifactory (which will hopefully be backported to JANA2 proper) we have a method signature that clang-tidy rightfully complains about:

void Process(int64_t run_number, uint64_t event_number) override;

One idea is to create strong types for run number and event number. This would clean up a bunch of code that uses these numbers.

Another idea is to add support for arbitrary event key levels. For each level there would be a corresponding Change*() callback. This would let us support things like

DraTeots commented 7 months ago

I would argue against strong types for run_number and event_number. One might think that strong types add code clarity. But in reality it might work on opposite. When you read the type name - it is yet another type you have, that requires you to scroll, find origins and digest. You read something like RunNumber_t run_number or run_number_t run_number and is... like, is it uint64_t? Is it struct? Some alias? What is it? Should I know how to use it?

There are definitely cases when it is beneficial to make strong types based on base types. Like imagine having an Angle type in geometry, which, being basically an alias to double, might be used everywhere. And really bring clarity when you see Angle a, b. And bring extended functionality for working with angles.

But for int64_t run_number and uint64_t event_number, which are actually not used widely, clearly defined, not requiring additional functions, IMO, the tradeoff is not on strong types side.

nathanwbrei commented 3 days ago

Closing this issue because we won't be using strong types. Instead of strong types, the JOmniFactory type signature will eventually accept a JEventKey that understands the full event hierarchy.