JeffersonLab / analyzer

HallA C++ Analyzer
BSD 3-Clause "New" or "Revised" License
7 stars 54 forks source link

[WIP] Does this interface make sense? #163

Closed whit2333 closed 6 years ago

whit2333 commented 6 years ago

https://github.com/JeffersonLab/analyzer/blob/05bb0df3698109a458024f0185a3107c3715cfe2/src/THaCutList.h#L42

Why is this a const THaVarList * instead of just a non-const? Is it a built-in assumption that the argument is a global variable?

hansenjo commented 6 years ago

THaCutList does not modify the list of variables, so it can be declared const within this class and in function signatures.

The variable list given to THaCutList does not have to be a global variable. It can be anything, local, thread-local, whatever. The const just means that THaCutList will not change the variable list (i.e. add/delete variables).

Should THaCutList be allowed to change the variable list? In the original design, no. Cuts were envisioned to be consumers of variables, not possible creators of them. So the interface made excellent sense.

But this assumption may not be good for new use cases. Do you have anything in mind? Propagate cuts into the variable list? What would be the benefit?

whit2333 commented 6 years ago

I ran into this when I was debugging something related to the global variables. One of the classes had a default constructor argument that was a global variable.

I'll follow up on this later when I have more time.

hansenjo commented 6 years ago

Continued as issue #166.