FGT SW Review: StFgtConsts.h
File: StFgtUtil/StFgtConsts.h
• What is the reason for the enum values to be in an enum and not just constants?
We prefer enums. Other detectors do it. It saves having to have a .cxx file.
• The enum value names themselves follow what seems to the de facto STAR convention (start lower case k) so thatʼs good. The enumerations are all unnamed; I generally prefer them named something informative (so functions that should only take those values as input can have that as their argument type, not int).
Naming them is unneeded. They aren't passed as input to functions--they are generally used as limits of loops or in math equations. One never needs to know the type, only the value.
• Not a fan of global variables. I would prefer them to be in a namespace or a class.
This extra overhead is not needed. We followed the patterns of other detectors.
• Not at all a fan of #define for constants.
Sorry you do not like. It was our choice, and STAR had no requirements one way or the other. It saves having to have a .cxx file. Others to it also.
• Why are integer constants in StFgtGeom instead of here? Or, why are these constants here and not in StFgtGeom? What is special about integer constants that means they logically associate with StFgtGeom and the float constants donʼt? It seems that with the current code structure, constants are spread across three sources (DB, StFgtConsts, StFgtGeom) and I wanted to understand the reasoning behind how they are divided.
This has been cleaned up. Values used in manly places are in StFgtConsts. The DB holds values which change. StFgtGeom holds values which are only used internal to it, or large tables.
Compilation warnings:
./StRoot/StFgtUtil/StFgtConsts.h:39: error: comma at end of enumerator list
Actually, it is not the end. If you look at it, it is fine. There's a multiline comment which tricks your -pedantic option.
- sgliske's blog
- Login or register to post comments