New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DQM: header and namespace cleanup #27476
Changes from all commits
c6919ac
69d0e8a
3021888
e3e94e6
264d5a3
ef9736b
333bb1e
c66102c
a1a38a0
9579d45
f8bba30
8edd4a6
c8f5fb2
1ef05ac
37df42e
cc6db08
be08300
fd99101
dd9a721
775e359
ec99408
e4f8eb0
e3b727a
29c78a4
7233436
07f47c0
785d00e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,22 +129,21 @@ void SiPixelErrorsDigisToCalibDigis::endJob() { | |
|
||
// ------------ helper functions --------------------------------------------------------- | ||
|
||
MonitorElement* SiPixelErrorsDigisToCalibDigis::bookDQMHistogram2D(uint32_t detid, | ||
std::string name, | ||
std::string title, | ||
int nchX, | ||
double lowX, | ||
double highX, | ||
int nchY, | ||
double lowY, | ||
double highY) { | ||
SiPixelErrorsDigisToCalibDigis::MonitorElement* SiPixelErrorsDigisToCalibDigis::bookDQMHistogram2D(uint32_t detid, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m fairly certain you could have used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably yes, but that requires changes in two places (header and implementation). Also the return type might not (always) be obvious, so I went the most obvious route. |
||
std::string name, | ||
std::string title, | ||
int nchX, | ||
double lowX, | ||
double highX, | ||
int nchY, | ||
double lowY, | ||
double highY) { | ||
std::string hid = theHistogramIdWorker_->setHistoId(name, detid); | ||
return daqBE_->book2D(hid, title, nchX, lowX, highX, nchY, lowY, highY); | ||
} | ||
|
||
MonitorElement* SiPixelErrorsDigisToCalibDigis::bookDQMHistoPlaquetteSummary2D(uint32_t detid, | ||
std::string name, | ||
std::string title) { | ||
SiPixelErrorsDigisToCalibDigis::MonitorElement* SiPixelErrorsDigisToCalibDigis::bookDQMHistoPlaquetteSummary2D( | ||
uint32_t detid, std::string name, std::string title) { | ||
DetId detId(detid); | ||
const TrackerGeometry& theTracker(*geom_); | ||
const PixelGeomDetUnit* theGeomDet = dynamic_cast<const PixelGeomDetUnit*>(theTracker.idToDet(detId)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to keep in mind for the future. We highly encourage the use of
using
instead oftypedef
in all future code development.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used
using
in some places, but then switched totypedef
for symmetry with those cases where the declarations are put in namespaces (fewer, but still some).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@schneiml could you please clarify the rationale behind this choice? Is there a specific reason I am missing, or is it mostly a matter of taste?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding typedef vs. using:
using
only works in class scopes, but some of these declarations also appear in namespaces, where afaikusing
can't be used. So I chose to use the form of declaration that works everywhere.Another option is to not import the names but just use full names. I also did that in a few places where the declarations caused trouble (e.g. due to multi-inheritance), but in general I preferred to have the import since it leads to less changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using
can also be used outside of class scope.