Skip to content

Commit

Permalink
Containers: renamed String::strip{Pre,Suf}fix() to except{Pre,Suf}fix().
Browse files Browse the repository at this point in the history
Because the original naming confusingly made the following code look
like it should be doing something -- while it shouldn't:

    Containers::String a = "not confused!";

    a.stripPrefix("not "); // does not mutate, returns a view

Same for StringView as well. Even though the classes are
"relatively" new and not part of any release yet, the original APIs are
kept and marked as deprecated -- I don't want people to suffer
breakages.
  • Loading branch information
mosra committed Feb 21, 2021
1 parent 28cdde0 commit b310615
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 93 deletions.
16 changes: 8 additions & 8 deletions src/Corrade/Containers/String.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -491,20 +491,20 @@ bool String::hasSuffix(const StringView suffix) const {
return StringView{*this}.hasSuffix(suffix);
}

MutableStringView String::stripPrefix(const StringView prefix) & {
return MutableStringView{*this}.stripPrefix(prefix);
MutableStringView String::exceptPrefix(const StringView prefix) & {
return MutableStringView{*this}.exceptPrefix(prefix);
}

StringView String::stripPrefix(const StringView prefix) const & {
return StringView{*this}.stripPrefix(prefix);
StringView String::exceptPrefix(const StringView prefix) const & {
return StringView{*this}.exceptPrefix(prefix);
}

MutableStringView String::stripSuffix(const StringView suffix) & {
return MutableStringView{*this}.stripSuffix(suffix);
MutableStringView String::exceptSuffix(const StringView suffix) & {
return MutableStringView{*this}.exceptSuffix(suffix);
}

StringView String::stripSuffix(const StringView suffix) const & {
return StringView{*this}.stripSuffix(suffix);
StringView String::exceptSuffix(const StringView suffix) const & {
return StringView{*this}.exceptSuffix(suffix);
}

char* String::release() {
Expand Down
70 changes: 58 additions & 12 deletions src/Corrade/Containers/String.h
Original file line number Diff line number Diff line change
Expand Up @@ -614,37 +614,83 @@ class CORRADE_UTILITY_EXPORT String {
* @brief Whether the string begins with given prefix
*
* Equivalent to @ref BasicStringView::hasPrefix().
* @see @ref stripPrefix()
* @see @ref exceptPrefix()
*/
bool hasPrefix(StringView prefix) const;

/**
* @brief Whether the string ends with given suffix
*
* Equivalent to @ref BasicStringView::hasSuffix().
* @see @ref stripSuffix()
* @see @ref exceptSuffix()
*/
bool hasSuffix(StringView suffix) const;

/**
* @brief Strip given prefix
* @brief View with given prefix stripped
*
* Equivalent to @ref BasicStringView::stripPrefix(). Not allowed to be
* called on a rvalue since the returned view would become dangling.
* Equivalent to @ref BasicStringView::exceptPrefix(). Not allowed to
* be called on a r-value since the returned view would become
* dangling.
* @see @ref hasPrefix()
*/
MutableStringView stripPrefix(StringView prefix) &;
StringView stripPrefix(StringView prefix) const &; /**< @overload */
MutableStringView exceptPrefix(StringView prefix) &;
StringView exceptPrefix(StringView prefix) const &; /**< @overload */

#ifdef CORRADE_BUILD_DEPRECATED
/**
* @brief Strip given prefix
* @brief @copybrief exceptPrefix()
* @m_deprecated_since_latest Deprecated due to confusing naming that
* could imply the original instance gets modified. Use
* @ref exceptPrefix() instead.
*/
CORRADE_DEPRECATED("use exceptPrefix() instead") MutableStringView stripPrefix(StringView prefix) & {
return exceptPrefix(prefix);
}

/**
* @brief @copybrief exceptPrefix()
* @m_deprecated_since_latest Deprecated due to confusing naming that
* could imply the original instance gets modified. Use
* @ref exceptPrefix() instead.
*/
CORRADE_DEPRECATED("use exceptPrefix() instead") StringView stripPrefix(StringView prefix) const & {
return exceptPrefix(prefix);
}
#endif

/**
* @brief View with given suffix stripped
*
* Equivalent to @ref BasicStringView::stripSuffix(). Not allowed to be
* called on a rvalue since the returned view would become dangling.
* Equivalent to @ref BasicStringView::exceptSuffix(). Not allowed to
* be called on a r-value since the returned view would become
* dangling.
* @see @ref hasSuffix()
*/
MutableStringView stripSuffix(StringView suffix) &;
StringView stripSuffix(StringView suffix) const &; /**< @overload */
MutableStringView exceptSuffix(StringView suffix) &;
StringView exceptSuffix(StringView suffix) const &; /**< @overload */

#ifdef CORRADE_BUILD_DEPRECATED
/**
* @brief @copybrief exceptSuffix()
* @m_deprecated_since_latest Deprecated due to confusing naming that
* could imply the original instance gets modified. Use
* @ref exceptSuffix() instead.
*/
CORRADE_DEPRECATED("use exceptSuffix() instead") MutableStringView stripSuffix(StringView suffix) & {
return exceptSuffix(suffix);
}

/**
* @brief @copybrief exceptSuffix()
* @m_deprecated_since_latest Deprecated due to confusing naming that
* could imply the original instance gets modified. Use
* @ref exceptSuffix() instead.
*/
CORRADE_DEPRECATED("use exceptSuffix() instead") StringView stripSuffix(StringView suffix) const & {
return exceptSuffix(suffix);
}
#endif

/**
* @brief Release data storage
Expand Down
8 changes: 4 additions & 4 deletions src/Corrade/Containers/StringView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,15 +290,15 @@ template<class T> bool BasicStringView<T>::hasSuffix(const StringView suffix) co
return std::strncmp(_data + size - suffixSize, suffix.data(), suffixSize) == 0;
}

template<class T> BasicStringView<T> BasicStringView<T>::stripPrefix(const StringView prefix) const {
template<class T> BasicStringView<T> BasicStringView<T>::exceptPrefix(const StringView prefix) const {
CORRADE_ASSERT(hasPrefix(prefix),
"Containers::StringView::stripPrefix(): string doesn't begin with" << prefix, {});
"Containers::StringView::exceptPrefix(): string doesn't begin with" << prefix, {});
return suffix(prefix.size());
}

template<class T> BasicStringView<T> BasicStringView<T>::stripSuffix(const StringView suffix) const {
template<class T> BasicStringView<T> BasicStringView<T>::exceptSuffix(const StringView suffix) const {
CORRADE_ASSERT(hasSuffix(suffix),
"Containers::StringView::stripSuffix(): string doesn't end with" << suffix, {});
"Containers::StringView::exceptSuffix(): string doesn't end with" << suffix, {});
return prefix(size() - suffix.size());
}

Expand Down
36 changes: 30 additions & 6 deletions src/Corrade/Containers/StringView.h
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ template<class T> class CORRADE_UTILITY_EXPORT BasicStringView {
*
* For an empty string returns @cpp true @ce only if @p prefix is empty
* as well.
* @see @ref stripPrefix()
* @see @ref exceptPrefix()
*/
bool hasPrefix(StringView prefix) const;

Expand All @@ -525,12 +525,12 @@ template<class T> class CORRADE_UTILITY_EXPORT BasicStringView {
*
* For an empty string returns @cpp true @ce only if @p suffix is empty
* as well.
* @see @ref stripSuffix()
* @see @ref exceptSuffix()
*/
bool hasSuffix(StringView suffix) const;

/**
* @brief Strip given prefix
* @brief View with given prefix stripped
*
* Expects that the string actually begins with given prefix. The
* function uses @ref slice() internally, meaning it propagates the
Expand All @@ -539,10 +539,22 @@ template<class T> class CORRADE_UTILITY_EXPORT BasicStringView {
* the view always points to existing memory.
* @see @ref hasPrefix()
*/
BasicStringView<T> stripPrefix(StringView prefix) const;
BasicStringView<T> exceptPrefix(StringView prefix) const;

#ifdef CORRADE_BUILD_DEPRECATED
/**
* @brief Strip given suffix
* @brief @copybrief exceptPrefix()
* @m_deprecated_since_latest Deprecated due to confusing naming that
* could imply the original instance gets modified. Use
* @ref exceptPrefix() instead.
*/
CORRADE_DEPRECATED("use exceptPrefix() instead") BasicStringView<T> stripPrefix(StringView prefix) const {
return exceptPrefix(prefix);
}
#endif

/**
* @brief View with given suffix stripped
*
* Expects that the string actually ends with given suffix. The
* function uses @ref slice() internally, meaning it propagates the
Expand All @@ -551,7 +563,19 @@ template<class T> class CORRADE_UTILITY_EXPORT BasicStringView {
* the view always points to existing memory.
* @see @ref hasSuffix()
*/
BasicStringView<T> stripSuffix(StringView suffix) const;
BasicStringView<T> exceptSuffix(StringView suffix) const;

#ifdef CORRADE_BUILD_DEPRECATED
/**
* @brief @copybrief exceptSuffix()
* @m_deprecated_since_latest Deprecated due to confusing naming that
* could imply the original instance gets modified. Use
* @ref exceptSuffix() instead.
*/
CORRADE_DEPRECATED("use exceptSuffix() instead") BasicStringView<T> stripSuffix(StringView suffix) const {
return exceptSuffix(suffix);
}
#endif

private:
/* Needed for mutable/immutable conversion */
Expand Down
54 changes: 27 additions & 27 deletions src/Corrade/Containers/Test/StringTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ struct StringTest: TestSuite::Tester {
void hasPrefix();
void hasSuffix();

void stripPrefix();
void stripPrefixInvalid();
void stripSuffix();
void stripSuffixInvalid();
void exceptPrefix();
void exceptPrefixInvalid();
void exceptSuffix();
void exceptSuffixInvalid();

void release();

Expand Down Expand Up @@ -235,10 +235,10 @@ StringTest::StringTest() {
&StringTest::hasPrefix,
&StringTest::hasSuffix,

&StringTest::stripPrefix,
&StringTest::stripPrefixInvalid,
&StringTest::stripSuffix,
&StringTest::stripSuffixInvalid,
&StringTest::exceptPrefix,
&StringTest::exceptPrefixInvalid,
&StringTest::exceptSuffix,
&StringTest::exceptSuffixInvalid,

&StringTest::release,

Expand Down Expand Up @@ -1463,19 +1463,19 @@ void StringTest::hasSuffix() {
CORRADE_VERIFY(!String{"overcomplicated"}.hasSuffix("somplicated"));
}

void StringTest::stripPrefix() {
void StringTest::exceptPrefix() {
/* These rely on StringView conversion and then delegate there so we don't
need to verify SSO behavior, only the basics and flag propagation */

String a{"overcomplicated"};
CORRADE_COMPARE(a.stripPrefix("over"), "complicated"_s);
CORRADE_COMPARE(a.stripPrefix("over").flags(), StringViewFlag::NullTerminated);
CORRADE_COMPARE(a.exceptPrefix("over"), "complicated"_s);
CORRADE_COMPARE(a.exceptPrefix("over").flags(), StringViewFlag::NullTerminated);

const String ca{"overcomplicated"};
CORRADE_COMPARE(ca.stripPrefix("over"), "complicated");
CORRADE_COMPARE(ca.exceptPrefix("over"), "complicated");
}

void StringTest::stripPrefixInvalid() {
void StringTest::exceptPrefixInvalid() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
Expand All @@ -1485,29 +1485,29 @@ void StringTest::stripPrefixInvalid() {

std::ostringstream out;
Error redirectOutput{&out};
a.stripPrefix("complicated");
ca.stripPrefix("complicated");
a.exceptPrefix("complicated");
ca.exceptPrefix("complicated");
/* Assert is coming from StringView */
CORRADE_COMPARE(out.str(),
"Containers::StringView::stripPrefix(): string doesn't begin with complicated\n"
"Containers::StringView::stripPrefix(): string doesn't begin with complicated\n");
"Containers::StringView::exceptPrefix(): string doesn't begin with complicated\n"
"Containers::StringView::exceptPrefix(): string doesn't begin with complicated\n");
}

void StringTest::stripSuffix() {
void StringTest::exceptSuffix() {
/* These rely on StringView conversion and then delegate there so we don't
need to verify SSO behavior, only the basics and flag propagation */

String a{"overcomplicated"};

CORRADE_COMPARE(a.stripSuffix("complicated"), "over"_s);
CORRADE_COMPARE(a.stripSuffix("complicated").flags(), StringViewFlags{});
CORRADE_COMPARE(a.stripSuffix("").flags(), StringViewFlag::NullTerminated);
CORRADE_COMPARE(a.exceptSuffix("complicated"), "over"_s);
CORRADE_COMPARE(a.exceptSuffix("complicated").flags(), StringViewFlags{});
CORRADE_COMPARE(a.exceptSuffix("").flags(), StringViewFlag::NullTerminated);

const String ca{"overcomplicated"};
CORRADE_COMPARE(ca.stripSuffix("complicated"), "over");
CORRADE_COMPARE(ca.exceptSuffix("complicated"), "over");
}

void StringTest::stripSuffixInvalid() {
void StringTest::exceptSuffixInvalid() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
Expand All @@ -1517,12 +1517,12 @@ void StringTest::stripSuffixInvalid() {

std::ostringstream out;
Error redirectOutput{&out};
a.stripSuffix("over");
ca.stripSuffix("over");
a.exceptSuffix("over");
ca.exceptSuffix("over");
/* Assert is coming from StringView */
CORRADE_COMPARE(out.str(),
"Containers::StringView::stripSuffix(): string doesn't end with over\n"
"Containers::StringView::stripSuffix(): string doesn't end with over\n");
"Containers::StringView::exceptSuffix(): string doesn't end with over\n"
"Containers::StringView::exceptSuffix(): string doesn't end with over\n");
}

void StringTest::release() {
Expand Down
Loading

0 comments on commit b310615

Please sign in to comment.