added initialization to CRTHit data members #161
Conversation
mt82
changed the title
kjplows
left a comment
kjplows
left a comment
There was a problem hiding this comment.
Looks good to me thanks! I'll let @PetrilloAtWork decide if using flavours of 0 is better than calling on std::numeric_limits<T> here; my one comment is if any CRT variables are expected to be 0 physically (looking at ts0_ns, x_pos etc) probably std::numeric_limits::lowest() is easier to interpret as nonsense.
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Stupid GitHub ate my comment.
Which boiled down to: agree with @kjplows, define static constexpr double kInvalidD = std::numeric_limits<double>::lowest(); and kInvalidF for float, use those to initialise, and use -1 as plane value.
"Stupid GitHub" was an euphemism, by the way.
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Stupid GitHub ate my comment.
Which boiled down to: agree with @kjplows, define static constexpr double kInvalidD = std::numeric_limits<double>::lowest(); and kInvalidF for float, use those to initialise, and use -1 as plane value.
"Stupid GitHub" was an euphemism, by the way.
…: plain -1 for integer-like variables and std::numeric_limits<>::lowest() for the others
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Approved, with my thanks.
|
trigger build ci_ref=v10_14_00_01 LArSoft/larpandoracontent@LARSOFT_SUITE_v10_14_02_01 LArSoft/larpandora@LARSOFT_SUITE_v10_14_02_01 LArSoft/larreco@LARSOFT_SUITE_v10_14_02_01 LArSoft/larrecodnn@LARSOFT_SUITE_v10_14_02_01 LArSoft/larana@LARSOFT_SUITE_v10_14_02_01 LArSoft/lareventdisplay@LARSOFT_SUITE_v10_14_02_01 LArSoft/larsoft@LARSOFT_SUITE_v10_14_02_01 LArSoft/lar*@LARSOFT_SUITE_v10_14_02 SBNSoftware/sbndaq-artdaq-core@v1_10_06 SBNSoftware/sbn*@SBN_SUITE_v10_14_02 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v10_14_02 SBNSoftware/sbncode@v10_14_02 SBNSoftware/sbn*@SBN_SUITE_v10_14_02 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
Default initialization values have been introduced for all CRTHit data members. This change ensures that variables are properly initialized at object creation, reducing the risk of undefined behavior and improving code reliability.