Skip to content
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.

Sign up for GitHub

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

Updated TRestMetadata::GetFieldValue to allow for default value #277

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Updated TRestMetadata::GetFieldValue to allow for default value #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
962e0f9
Updated TRestMetadata::GetFieldValue to allow for default value
lobis Jul 29, 2022
f372ab4
TRestStringHelper::StringToBool - now also returns the result of `std…
lobis Jul 29, 2022
38fae37
TRestMetadata - fixed bad const
lobis Jul 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion source/framework/core/inc/TRestMetadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ class TRestMetadata : public TNamed {

protected:
// new xml utilities
std::string GetFieldValue(std::string parName, TiXmlElement* e);
std::string GetFieldValue(const std::string& parName, const TiXmlElement* e,
const std::string& defaultValue = "Not defined") const;
std::string GetParameter(std::string parName, TiXmlElement* e,
TString defaultValue = PARAMETER_NOT_FOUND_STR);
Double_t GetDblParameterWithUnits(std::string parName, TiXmlElement* e,
Expand Down
7 changes: 4 additions & 3 deletions source/framework/core/src/TRestMetadata.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1548,14 +1548,15 @@ TVector3 TRestMetadata::Get3DVectorParameterWithUnits(std::string parName, TiXml
/// A version of GetParameter() but only find parameter in the fields of xml
/// element. If not found, the returned string is "Not defined"
///
std::string TRestMetadata::GetFieldValue(std::string parName, TiXmlElement* e) {
string TRestMetadata::GetFieldValue(const string& parName, const TiXmlElement* e,
const string& defaultValue) const {
if (e == nullptr) {
RESTDebug << "Element is null" << RESTendl;
return "Not defined";
return defaultValue;
}
const char* val = e->Attribute(parName.c_str());
if (val == nullptr) {
return "Not defined";
return defaultValue;
}

string result = (string)val;
Expand Down
2 changes: 1 addition & 1 deletion source/framework/tools/inc/TRestStringHelper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Double_t StringToDouble(std::string in);
Int_t StringToInteger(std::string in);
std::string IntegerToString(Int_t n, std::string format = "%d");
std::string DoubleToString(Double_t d, std::string format = "%4.2lf");
Bool_t StringToBool(std::string in);
Bool_t StringToBool(const std::string& in);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compilation failure is due to this const keyword. It makes the argument unmatched. If we think this change is necessary then we shall open a new PR to replace all the AddConverter(xxx) with const-input methods.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be implemented if it's not too much work, its not high priority but using const T& should always be used to pass non built-in types or atleast be the default option, so I think we should support it.

Long64_t StringToLong(std::string in);
TVector3 StringTo3DVector(std::string in);
TVector2 StringTo2DVector(std::string in);
Expand Down
4 changes: 2 additions & 2 deletions source/framework/tools/src/TRestStringHelper.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,8 @@ string REST_StringHelper::IntegerToString(Int_t n, std::string format) { return
///
string REST_StringHelper::DoubleToString(Double_t d, std::string format) { return Form(format.c_str(), d); }

Bool_t REST_StringHelper::StringToBool(std::string in) {
return (ToUpper(in) == "TRUE" || ToUpper(in) == "ON");
Bool_t REST_StringHelper::StringToBool(const std::string& in) {
return (ToUpper(in) == "TRUE" || ToUpper(in) == "ON" || ToUpper(in) == "1");
}

Long64_t REST_StringHelper::StringToLong(std::string in) {
Expand Down