Feature: JParameterManager comma escaping #401
Conversation
| std::string t; | ||
| if (s.find('\\') != std::string::npos) { | ||
| s.erase(s.find('\\')); | ||
| temp += s + ','; |
There was a problem hiding this comment.
When find yourself concatenating a string inside a loop, you should probably be using a string builder instead, because it will have much better time complexity. In C++ you would use std::ostringstream
There was a problem hiding this comment.
Never mind, this might not be a problem in std::string. It certainly was a problem in other contexts such as java's String class
| @@ -369,12 +373,39 @@ template<typename T, size_t N> | |||
| inline void JParameterManager::Parse(const std::string& value,std::array<T,N> &val) { | |||
| std::string s; | |||
| std::stringstream ss(value); | |||
| std::string temp = ""; // creating a temp var allows us to store the string s where the escape character was used | |||
There was a problem hiding this comment.
This looks like it's a leftover from earlier work. Let's just remove this
| @@ -383,13 +414,40 @@ inline void JParameterManager::Parse(const std::string& value, std::vector<T> &v | |||
| std::stringstream ss(value); | |||
| std::string s; | |||
| val.clear(); // clearing the input vector to ensure no dulication which can be caused due to val.push_back(t); | |||
| std::string temp = ""; // creating a temp var allows us to store the string s where the escape character was used | |||
| std::string temp = ""; // creating a temp var allows us to store the string s where the escape character was used | ||
| while (getline(ss, s, ',')) { | ||
| std::string t; | ||
| if (s.find('\\') != std::string::npos) { |
There was a problem hiding this comment.
This finds and removes the first escape character anywhere in the string, not necessarily at the end. I'd replace this with simply checking whether the last chararacter is a \\, which is much faster, anyhow.
Test case: "All of this belongs in\t slot zero\, including the tab character,whereas all of this belongs in slot one"
| REQUIRE(param->GetValue() == "theta-fmod(phi-fmod(phi\\,5)\\,7),theta-fmod(theta\\,10),omega-fmod(omega\\,15)"); | ||
|
|
||
| std::array<std::string,3> temp; | ||
| jpm.Parse(jpm.Stringify("theta-fmod(phi-fmod(phi\\,5)\\,7),theta-fmod(theta\\,10),omega-fmod(omega\\,15)"), temp); |
There was a problem hiding this comment.
Here you are stringifying a string, you should be stringifying a vector/array
This addresses issue #380