Difference: LHCbID (5 vs. 6)

Revision 62005-09-14 - GloriaCorti

Line: 1 to 1
 
META TOPICPARENT name="EventModelReview"

LHCbID Class

Line: 70 to 70
 it more as the latter than the former. Would this be a fair comment ? -- ChrisRJones - 25 Aug 2005
Added:
>
>
I agree that it should be clarified if the LHCbID class is a general LHCb class, or it is a utility class for Tracking. If the latter I think to have there RICH and CALO is confusing. For the MUON one can argue it could be used for track refitting with MUON information. In any case I think it is important to have easy contruction of DetectorID from the LHCbID whatever detector it applies to. I noticed that a contructor exist for LHCbID from ITChannelID but then the accessor is called isST(). I think they should be consistent. -- GloriaCorti - 14 Sep 2005
 

Constructors from int and conversion to int

Another missing feature is the conversion to/from int, available for most/all channel ID, and useful when using this LHCbID in a Linker relation.

Line: 85 to 90
 The problem appears to be that the class the MuonTileID class is defined in MuonKernel package, unlike the other sub-detector channel ID classes which are all defined in LHCbKernel. MuonTileID could be moved to LHCbKernel, and as far as I can tell just looking at the code this would also require moving "MuonKernel/MuonBase.h" and "MuonKernel/MuonLayout.h" into LHCbKernel. Maybe with some more thought from the Muon Group this could be avoided, but it doesn't seem to much of a problem, particularly as it was also done for other sub-detectors, such as RichSmartID which required a few enum files to also be moved from RichKernel into LHCbKernel -- ChrisRJones - 22 Aug 2005
Added:
>
>
Does it require MuonLayout.h to be movedto LHCbKernel? It looks to me that it uses the IMuonLayout interface (implemented by MuonLayout). There is an implementation of MuonTileID that includes also MuonStationLayout.h and MuonSystemLayout.h: are those really necessary? Could MuonTileID depend only on IMuonLayout and MuonBase? To me it look like it should be or am I missing something? By looking at the code there is a forward declaration of MuonTileID in MuonTileID.h... Some tidying up of the code would be nice. -- GloriaCorti - 14 Sep 2005
 

"Magic Numbers"

A minor comment, but there are some magic numbers scattered throughout the class (Hardcoded numbers without any "obvious" meaning). It would help make the class clearer if these could be either removed (for example derived from more fundamental numbers, such as the number of bits for each field) or at least clearly documented so readers of the code can see how they where arrived at.

 
This site is powered by the TWiki collaboration platform Powered by PerlCopyright & 2008-2019 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.
Ideas, requests, problems regarding TWiki? Send feedback