Skip to content

GaussianSumFilter migrate to return by value.

The overall aims are better described at !31931 (merged)

  • If we return ownership of objects, we should use unique_ptr.
  • We'd prefer to return the vector by value, rather than returning an object on the heap.
  • We're returning a new object, but it's declared const. This doesn't really gain anything, and some downstream code was ending up doing a const_cast.

The last and first ones are partially covered by moving to MultiComponentState being

typedef std::pair<std::unique_ptr<Trk::TrackParameters>,double> ComponentParameters;
typedef std::vector<ComponentParameters> MultiComponentState;

from the rel 21

class MultiComponentState : public std::list<ComponentParameters>
class ComponentParameters : public std::pair<const TrackParameters*, double>{

i.e vector instead of list. But also unique_ptrTrk::TrackParameters rather than const TrackParameters* (the lack of const is also important for the future as allows in place updates rahter than creating new ). + other changes since then.

It is true though we would prefer to return these vectors directly. But std::unique_ptr<MultiComponentState> to MultiComponentState is not a stretch after we reached the current status.

When ownership/allocations are at play, it can help to see if things can be further simpified.

  • The changes on the headers are trivial std::unique_ptr<MultiComponentState> -> MultiComponentState

  • On the implementation side a check of the type if(!ptr_to_vec) becomes if (vec.empty()). i.e check for empty() return and conversely if(ptr_to_vec) --> if (!vec.empty())

  • The GsfExtrapolator is done bar the void Trk::GsfExtrapolator::extrapolateToVolumeBoundary and the one caller to it. This part has a bit not trivial handling of the "currentState" ptr (which have not figured 100% yet and will lead to a bigger change) ...

  • The GsfSmoother has been a bit messy on the points where we want to release to the EDM. This can be tackled perhaps a bit better although needs a bit thinking. But this is not where most of the calls / work is done. Still something to look at at the end.

  • The others were trivial and kind of consolidate the 3 targers for most of the internal tooling.

@amorley this now passed Tier0 tests (at least locally so do not expect any CI drama).

Edited by Christos Anastopoulos

Merge request reports