[ https://issues.apache.org/jira/browse/MINIFICPP-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adam Hunyadi updated MINIFICPP-1329: ------------------------------------ Summary: Fix implementation and usages of StringUtils::StringToBool (was: Fix usages of StringUtils::StringToBool) > Fix implementation and usages of StringUtils::StringToBool > ---------------------------------------------------------- > > Key: MINIFICPP-1329 > URL: https://issues.apache.org/jira/browse/MINIFICPP-1329 > Project: Apache NiFi MiNiFi C++ > Issue Type: Bug > Reporter: Adam Hunyadi > Priority: Minor > Labels: MiNiFi-CPP-Hygiene > > *Background:* > Conversions from string to other values in MINIFI usually follow the > convention of changing an output value and returning a boolean denoting the > success of the conversion. For booleans however, this is not the case: > {code:c++|title=Current Implementation} > bool StringUtils::StringToBool(std::string input, bool &output) { > std::transform(input.begin(), input.end(), input.begin(), ::tolower); > std::istringstream(input) >> std::boolalpha >> output; > return output; > } > {code} > It is known to be misused in the code, for example this code assumes the > return value false corresponds to a parse failure: > > [https://github.com/apache/nifi-minifi-cpp/blob/rel/minifi-cpp-0.7.0/extensions/opc/src/putopc.cpp#L319-L323] > *Proposal:* > If we want to stay consistent with the other conversions, we can do this: > {code:c++|title=Minimum change for the new implementation} > bool StringUtils::StringToBool(std::string input, bool &output) { > std::transform(input.begin(), input.end(), input.begin(), ::tolower); > output = "true" == input; > return output || "false" == input; > } > {code} > However, many cases use the return value as the conversion result. One should > be cautious: > # First we should copy the implementation to one with a different name > # Change the return value to void on the original > # Until the code compiles: > ## Eliminate all the usages of return values as parsed values > ## Redirect the checked value implementations to the copy > # Change the implementation of the original to return the conversion success > # Delete the copy > # Search and replace the name of the copy to the original > (i) With a bit more work, we can potentially change the return type to an > optional, or a success enum. -- This message was sent by Atlassian Jira (v8.3.4#803005)