[
https://issues.apache.org/jira/browse/MINIFICPP-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Adam Hunyadi updated MINIFICPP-1329:
------------------------------------
Description:
*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:
# Introduce the new implementation next to the old one as a function 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.
was:
*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 (your IDE can do
this, but verify the result)
(i) With a bit more work, we can potentially change the return type to an
optional, or a success enum.
> 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, beginner, newbie, starter
>
> *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:
> # Introduce the new implementation next to the old one as a function 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)
