Remove unused function and add more UTs

When browsing around the strutil files I found a function
that was never referenced inside the code base
"void StripString(string* s, const char* remove,
-                                 char replacewith);"

The name was kind of misleading as well and it seems like
it's a carbon copy of
"void ReplaceCharacters(string* s, const char* remove,
                                       char replacewith);"
(even the parameter names are the same, the code is the same..)

Is it intentional? Maybe for compatibility reasons? If so,
let's make it deprecated and use the ReplaceCharacters method inside
or the other way around.

Also, noticed there were no tests for "StripString" or "Replace".
Added some for both and planning on maybe making it more C++ish (?)
in another commit.
pull/6612/head
Andrei-Florin BENCSIK 5 years ago committed by Adam Cozzette
parent 95a1c4fbc4
commit 97b18021f6
  1. 15
      src/google/protobuf/stubs/strutil.cc
  2. 2
      src/google/protobuf/stubs/strutil.h
  3. 44
      src/google/protobuf/stubs/strutil_unittest.cc

@ -80,21 +80,6 @@ inline bool isprint(char c) {
return c >= 0x20 && c <= 0x7E; return c >= 0x20 && c <= 0x7E;
} }
// ----------------------------------------------------------------------
// StripString
// Replaces any occurrence of the character 'remove' (or the characters
// in 'remove') with the character 'replacewith'.
// ----------------------------------------------------------------------
void StripString(string* s, const char* remove, char replacewith) {
const char * str_start = s->c_str();
const char * str = str_start;
for (str = strpbrk(str, remove);
str != nullptr;
str = strpbrk(str + 1, remove)) {
(*s)[str - str_start] = replacewith;
}
}
// ---------------------------------------------------------------------- // ----------------------------------------------------------------------
// ReplaceCharacters // ReplaceCharacters
// Replaces any occurrence of the character 'remove' (or the characters // Replaces any occurrence of the character 'remove' (or the characters

@ -159,8 +159,6 @@ inline string StripSuffixString(const string& str, const string& suffix) {
// ---------------------------------------------------------------------- // ----------------------------------------------------------------------
PROTOBUF_EXPORT void ReplaceCharacters(string* s, const char* remove, PROTOBUF_EXPORT void ReplaceCharacters(string* s, const char* remove,
char replacewith); char replacewith);
PROTOBUF_EXPORT void StripString(string* s, const char* remove,
char replacewith);
PROTOBUF_EXPORT void StripWhitespace(string* s); PROTOBUF_EXPORT void StripWhitespace(string* s);

@ -836,6 +836,50 @@ TEST(StrCat, Ints) {
EXPECT_EQ(answer, "130"); EXPECT_EQ(answer, "130");
} }
class ReplaceChars : public ::testing::TestWithParam<std::tuple<string, string, const char*, char>> {
};
TEST_P(ReplaceChars, ReplacesAllOccurencesOfAnyCharInReplaceWithAReplaceChar) {
string expected = std::get<0>(GetParam());
string string_to_replace_in = std::get<1>(GetParam());
const char* what_to_replace = std::get<2>(GetParam());
char replacement = std::get<3>(GetParam());
ReplaceCharacters(&string_to_replace_in, what_to_replace, replacement);
ASSERT_EQ(expected, string_to_replace_in);
}
INSTANTIATE_TEST_CASE_P(Replace,
ReplaceChars,
::testing::Values(
std::make_tuple("", "", "", '_'), // empty string should remain empty
std::make_tuple(" ", " ", "", '_'), // no replacement string
std::make_tuple(" ", " ", "_-abcedf", '*'), // replacement character not in string
std::make_tuple("replace", "Replace", "R", 'r'), // replace one character
std::make_tuple("not_spaces__", "not\nspaces\t ", " \t\r\n", '_'), // replace some special characters
std::make_tuple("c++", "cxx", "x", '+'), // same character multiple times
std::make_tuple("qvvvvvng v T", "queueing a T", "aeiou", 'v'))); // replace all voewls
class StripWs: public ::testing::TestWithParam<std::tuple<string, string>> {
};
TEST_P(StripWs, AlwaysStripsLeadingAndTrailingWhitespace) {
string expected = std::get<0>(GetParam());
string string_to_strip = std::get<1>(GetParam());
StripWhitespace(&string_to_strip);
ASSERT_EQ(expected, string_to_strip);
}
INSTANTIATE_TEST_CASE_P(Strip,
StripWs,
::testing::Values(
std::make_tuple("", ""), // empty string should remain empty
std::make_tuple("", " "), // only ws should become empty
std::make_tuple("no whitespace", " no whitespace"), // leading ws removed
std::make_tuple("no whitespace", "no whitespace "), // trailing ws removed
std::make_tuple("no whitespace", " no whitespace "), // same nb. of leading and trailing
std::make_tuple("no whitespace", " no whitespace "), // different nb. of leading/trailing
std::make_tuple("no whitespace", " no whitespace "))); // more trailing than leading
} // anonymous namespace } // anonymous namespace
} // namespace protobuf } // namespace protobuf
} // namespace google } // namespace google

Loading…
Cancel
Save