LArSoft

AggregateVertex, VertexCheater, TrackCheater and EventMaker modules show non-optimal usage of associations:

larreco/VertexFinder/AggregateVertex_module.cc
larreco/VertexFinder/VertexCheater_module.cc
larreco/TrackFinder/TrackCheater_module.cc
larreco/EventFinder/EventMaker_module.cc

I notice dubious practises in AggregateVertex::MatchV2T(), VertexCheater::produce(), TrackCheater::produce() and EventMaker::produce().

The actions I recommend:
- since you use it, #include "art/Framework/Core/FindManyP.h"
- bring the queries FindManyP() out of the loops
- use const std::vector<art::Ptr<T>>& to store the result of FindManyP::at(), that returns a constant reference
- in TrackCheater::produce(), directly using a reference for hits is not possible; I'd recommend using a pointer pHits and after the value of the pointer is set, define a reference const std::vector<...>& hits = *pHits;; or, alternatively, use a temporary const vector reference inside the loop and create hits (also const vector reference) as fmh.at(p), of course declaring p in the scope containing the loop; this potentially avoids a number of copies

Contact person: Brian Rebel (brebel@fnal.gov)