[jira] [Updated] (MINIFICPP-1329) Fix implementation and usages of StringUtils::StringToBool
[ https://issues.apache.org/jira/browse/MINIFICPP-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ferenc Gerlits updated MINIFICPP-1329: -- Fix Version/s: 0.10.0 > 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 >Assignee: Amina Dinari >Priority: Minor > Labels: MiNiFi-CPP-Hygiene, beginner, newbie, starter > Fix For: 0.10.0 > > Time Spent: 7h > Remaining Estimate: 0h > > *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)
[jira] [Updated] (MINIFICPP-1329) Fix implementation and usages of StringUtils::StringToBool
[ 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
[jira] [Updated] (MINIFICPP-1329) Fix implementation and usages of StringUtils::StringToBool
[ https://issues.apache.org/jira/browse/MINIFICPP-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ferenc Gerlits 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: # 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. 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 (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 convers
[jira] [Updated] (MINIFICPP-1329) Fix implementation and usages of StringUtils::StringToBool
[ https://issues.apache.org/jira/browse/MINIFICPP-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Marton Szasz updated MINIFICPP-1329: Labels: MiNiFi-CPP-Hygiene beginner newbie starter (was: MiNiFi-CPP-Hygiene) > 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: > # 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)
[jira] [Updated] (MINIFICPP-1329) Fix implementation and usages of StringUtils::StringToBool
[ 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)