22.0-initialiseVariables-TrkTrack
initialise the pointer members at declaration, and protect against a nullptr for the m_trackStateVector in one instance.
Merge request reports
Activity
added Tracking master review-pending-level-1 labels
This is fine, but do we really want to allow Trk::Track without these ? i.e I assume they always need to have them no is which case we can check even earlier. Or we really have completely empty tracks?
Edited by Christos AnastopoulosHi Christos, thanks for taking a look! I came across this in the context of unit testing collections of Trk::Tracks. In this context, it is useful to be able to create a collection of default constructed (empty) tracks for insertion to a container and test the manipulations on that container. Given that the default constructor is not deleted, subsequent manipulations should either not fail or at least fail gracefully.
The default ctor should be used only in pool. And the point is in production prb you want to fail hard. As excluding the POOL, the way I understood the comments I read in the class (pre-existing) is that there are some invariants which have to be there.
I would have actually deleted the default ctor bar the comment
Track (); //!<needed by POOL. DO NOT USE YOURSELF!
because in some sense does not fit with anything else.
In this sense in production you "graceful" for the user would be a hard failure. i.e we should not have tracks violating
0041 * - A pointer to a const Trk::FitQuality - the fit quality of a track 0042 * - A pointer to a DataVector of const Trk::TrackStateOnSurface
as minimum.
This is what I find confusing. Of course we can make the default ctor protected since the cnv are friends. Which was going to be my suggestion.
But then if this is for unit tests is another bit of story.
Obviously the pool only ctor does not fit with the invariants of the class. So is an oddity, but not sure I want to add protection for this oddity ... or just make it so as to be usable only by the friend pool converters ..
E.g bar the POOL (which is special) no-one is supposed to create Tracks with this ctor.
Therefore wondering if we want to enforce this more. Or if it can happen, if being "silent" about it is the way to go ?
Edited by Christos Anastopoulos✅ CI Result SUCCESS (hash 6614ad4d)Athena AthSimulation AthGeneration AnalysisBase externals ✅ ✅ ✅ ✅ cmake ✅ ✅ ✅ ✅ make ✅ ✅ ✅ ✅ required tests ✅ ✅ ✅ ✅ optional tests ✅ ✅ ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
✅ AthGeneration: number of compilation errors 0, warnings 0
✅ AnalysisBase: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21940]added review-approved label and removed review-pending-level-1 label
mentioned in merge request !37348 (merged)
mentioned in commit dc54eafe
mentioned in merge request !37359 (merged)
added sweep:ignore label