framework icon indicating copy to clipboard operation
framework copied to clipboard

Updated TRestMetadata::GetFieldValue to allow for default value

Open lobis opened this issue 3 years ago • 1 comments

lobis Ok: 9

This PR was made to allow TRestMetadata::GetFieldValue to have a default value, instead of returning "Not defined".

However I could not make it work, there is some problem related to reflection I could not workout, not even creating an additional function GetFieldValueWithDefault that did this task...

I think this is necessary in order to avoid patterns like

bool isSensitive = false;
const string isSensitiveValue = GetFieldValue("sensitive", volumeDefinition);
if (isSensitiveValue != "Not defined") {
    isSensitive = StringToBool(isSensitiveValue);
}

which could be replaced by

const bool isSensitive = StringToBool(GetFieldValue("sensitive", volumeDefinition, "false"));

Could anyone (probably @nkx111) @rest-for-physics/core_dev take a look please? or perhaps there is an alternative (clean) way to achieve the same? Thanks

lobis avatar Jul 29 '22 19:07 lobis

It looks as the compilation error is related with Bool_t and bool types? Perhaps the const is causing problems here?

/builds/rest-for-physics/framework/source/framework/tools/inc/TRestReflector.h:471:84: error: invalid conversion from 'Bool_t (*)(const string&)' {aka 'bool (*)(const std::__cxx11::basic_string<char>&)'} to 'bool (*)(std::string)' {aka 'bool (*)(std::__cxx11::basic_string<char>)'} [-fpermissive]
[388](https://gitlab.cern.ch/rest-for-physics/framework/-/jobs/23650145#L388)

jgalan avatar Sep 22 '22 07:09 jgalan