search

JeffersonLab / analyzer

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

Global variables as default arguments #166

Closed whit2333 closed 6 years ago

whit2333 commented 6 years ago

https://github.com/JeffersonLab/analyzer/blob/2c3e5af0f071789aa9ed13c8a6a8b09aae864697/src/THaCut.h#L17

This cannot be a good idea.

Example: outside of "analyzer" it will break because the global variable is not initialized.

https://github.com/JeffersonLab/analyzer/blob/2c3e5af0f071789aa9ed13c8a6a8b09aae864697/src/THaInterface.cxx#L71

hansenjo commented 6 years ago

Explain why. It has never caused a hint of a problem in 15 years of production use.

hansenjo commented 6 years ago

I don't think there's a problem here. gHaVars and gHaCuts are initialized to null pointers during static initialization (have a look the definitions in lines 38-39 in THaInterface.cxx, not far at all from the line 71 that you quote, quite -quite- easy to find). Classes that use them as default values in their constructors (THaFormula, THaCut etc.) treat null pointers like empty lists and hence have well-defined behavior. Nothing "breaks."

Of course, giving empty lists to these classes is rather pointless. So, obviously, if an application wants to use them outside of the analyzer, it will have to set up and fill appropriate variable lists itself. It could use gHaVars/gHaCuts for this purpose, if global variables are suitable, and then take advantage of the default values in the constructors, or supply its own lists to override the defaults. All this works exactly as intended.

Incidentally, libHallA has not been designed to be used outside of the analyzer interactive environment. If that's an application you have in mind, it would be an unsupported configuration. I would not be surprised if certain things don't work quite right with that. You'd be welcome to file bug reports about such problems, just don't call such bugs "design flaws" or some such thing; they'd actually be user errors, if you will.