make statics const #1428
make statics const #1428
Conversation
|
A new Pull Request was created by @gartung (Patrick Gartung) for CMSSW_7_0_X. make statics const It involves the following packages: RecoMuon/TrackerSeedGenerator @nclopezo, @cmsbuild, @thspeer, @slava77 can you please review it and eventually sign? Thanks. |
|
I would think making these const member data would be better. |
| const double phiErrorScale{theConfig.getParameter<double>("phiErrorScale")}; | ||
| const double cotThetaErrorScale{theConfig.getParameter<double>("cotThetaErrorScale")}; | ||
| const double tipErrorScale{theConfig.getParameter<double>("tipErrorScale")}; | ||
| const double zipErrorScale{theConfig.getParameter<double>("zipErrorScale")}; |
There was a problem hiding this comment.
The question here is when this gets executed, and whether theConfig is already assigned. One simple solution would be to put it into the member initialization list in the ctor. Can you please do that?
There was a problem hiding this comment.
@gartung Patrick, this would mean changing the code in .h to
const double invPtErrorScale;
const double phiErrorScale;
...
and then in the .cc
L1MuonPixelTrackFitter::L1MuonPixelTrackFitter(const edm::ParameterSet& cfg)
: theConfig(cfg),
invPtErrorScale{theConfig.getParameter<double>("invPtErrorScale")},
phiErrorScale{theConfig.getParameter<double>("phiErrorScale")},
cotThetaErrorScale{theConfig.getParameter<double>("cotThetaErrorScale")},
tipErrorScale{theConfig.getParameter<double>("tipErrorScale")},
zipErrorScale{theConfig.getParameter<double>("zipErrorScale")}
{ }
There was a problem hiding this comment.
I did that be then the default ctor in the header has to be changed
- L1MuonPixelTrackFitter() {}
- L1MuonPixelTrackFitter();
or the compiler errors with uninitialized const members.
There was a problem hiding this comment.
Thanks for the changes.
This has a problem anyway, and also had it before, since theConfig was never assigned! It would have broken also before. We should get rid of that, or, if really needed, move to be private, and so unusable by anyone.
There was a problem hiding this comment.
The members are private. They are used in ::run() so need to be set to something.
There was a problem hiding this comment.
I meant the default constructor to be private. But if you can remove it, and it causes no problem, better like that. Thanks for the update.
|
Your previous change would have caused the default constructor to throw an exception. In fact, it looks like the old code would fail the same way. Try removing the default constructor all together. |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it? |
|
This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it? |
…erator Multithreading fixes -- Make statics const in RecoMuon/TrackerSeedGenerator
No description provided.