Re: [boost] [Boost-commit] svn:boost r81360 - in trunk: boost/algorithm libs/algorithm/test
AMDG On 11/15/2012 11:45 AM, marshall@idio.com wrote:
<snip> + // construct/copy + BOOST_CONSTEXPR basic_string_ref () +#ifdef BOOST_NO_CXX11_NULLPTR + : ptr_(NULL), len_(0) {} +#else + : ptr_(nullptr), len_(0) {} +#endif
Really? The two branches have exactly the same effect. The only reason to prefer nullptr is aesthetic and this is more than offset by the #ifdef.
+ + basic_string_ref(const charT* str) + : ptr_(str), len_(std::strlen(str)) {} +
strlen only works for char. You need to use traits::length
+#ifndef BOOST_NO_CXX11_HDR_INITIALIZER_LIST +// !! How do I do this? Look how initializer_lists work! + basic_string_ref(std::initializer_list<charT> il); // TODO +#endif +
Although this can be implemented, it's incredibly dangerous, because the string_ref will be left dangling when the initializer_list goes out of scope.
+ + // iterators + BOOST_CONSTEXPR const_iterator begin() const { return ptr_; } + BOOST_CONSTEXPR const_iterator cbegin() const { return ptr_; } + BOOST_CONSTEXPR const_iterator end() const { return ptr_ + len_; } + BOOST_CONSTEXPR const_iterator cend() const { return ptr_ + len_; } + const_reverse_iterator rbegin() const { return const_reverse_iterator (end()); } + const_reverse_iterator crbegin() const { return const_reverse_iterator (end()); } + const_reverse_iterator rend() const { return const_reverse_iterator (begin()); } + const_reverse_iterator crend() const { return const_reverse_iterator (begin()); } +
Check for tabs?
+ // capacity + BOOST_CONSTEXPR size_type max_size() const { return len_; }
I'm not convinced that this definition is correct. Returns: the size of the largest possible string. Note that this does not say that it returns the maximum size that this particular string_ref object can hold without reassignment.
+ + int compare(basic_string_ref x) const { + int cmp = std::memcmp ( ptr_, x.ptr_, std::min(len_, x.len_)); + return cmp != 0 ? cmp : ( len_ == x.len_ ? 0 : len_ < x.len_ ? -1 : 1 ); + } + + bool starts_with(charT c) const { return !empty() && front() == c; } + bool starts_with(basic_string_ref x) const { + return len_ >= x.len_ && std::memcmp ( ptr_, x.ptr_, x.len_ ) == 0; + } + + bool ends_with(charT c) const { return !empty() && back() == c; } + bool ends_with(basic_string_ref x) const { + return len_ >= x.len_ && std::memcmp ( ptr_ + len_ - x.len_, x.ptr_, x.len_ ) == 0; + } +
You can only use memcmp for char. You have to use traits::compare here.
+ +// Have to use traits here + size_type find(basic_string_ref s) const { + const_iterator iter = std::find_if ( this->cbegin (), this->cend (), + s.cbegin (), s.cend (), traits::eq ); + return iter = this->cend () ? npos : std::distance ( this->cbegin (), iter ); + } +
I think you mean std::search, not std::find_if.
+ size_type find(charT c) const { +#ifdef BOOST_NO_CXX11_LAMBDAS + const_iterator iter = std::find_if ( this->cbegin (), this->cend (), + detail::string_ref_traits_eq<charT, traits> ( c )); +#else + const_iterator iter = std::find_if ( this->cbegin (), this->cend (), + [c] ( charT val ) { return traits::eq ( c, val ); } ); +#endif + return iter == this->cend () ? npos : std::distance ( this->cbegin (), iter ); + } +
As with NULL/nulptr, I don't see the point of using C++11 lambdas if you have to #ifdef it with a C++03 implementation. In Christ, Steven Watanabe
On Nov 15, 2012, at 1:32 PM, Steven Watanabe <watanabesj@gmail.com> wrote:
AMDG
Thanks for the review!
On 11/15/2012 11:45 AM, marshall@idio.com wrote:
<snip> + // construct/copy + BOOST_CONSTEXPR basic_string_ref () +#ifdef BOOST_NO_CXX11_NULLPTR + : ptr_(NULL), len_(0) {} +#else + : ptr_(nullptr), len_(0) {} +#endif
Really? The two branches have exactly the same effect. The only reason to prefer nullptr is aesthetic and this is more than offset by the #ifdef.
Yeah.
+ + basic_string_ref(const charT* str) + : ptr_(str), len_(std::strlen(str)) {} +
strlen only works for char. You need to use traits::length
Fixed.
+#ifndef BOOST_NO_CXX11_HDR_INITIALIZER_LIST +// !! How do I do this? Look how initializer_lists work! + basic_string_ref(std::initializer_list<charT> il); // TODO +#endif +
Although this can be implemented, it's incredibly dangerous, because the string_ref will be left dangling when the initializer_list goes out of scope.
Still there.
+ // iterators + BOOST_CONSTEXPR const_iterator begin() const { return ptr_; } + BOOST_CONSTEXPR const_iterator cbegin() const { return ptr_; } + BOOST_CONSTEXPR const_iterator end() const { return ptr_ + len_; } + BOOST_CONSTEXPR const_iterator cend() const { return ptr_ + len_; } + const_reverse_iterator rbegin() const { return const_reverse_iterator (end()); } + const_reverse_iterator crbegin() const { return const_reverse_iterator (end()); } + const_reverse_iterator rend() const { return const_reverse_iterator (begin()); } + const_reverse_iterator crend() const { return const_reverse_iterator (begin()); } +
Check for tabs?
When I de-tabbed it, it got messed up.
+ // capacity + BOOST_CONSTEXPR size_type max_size() const { return len_; }
I'm not convinced that this definition is correct. Returns: the size of the largest possible string.
Note that this does not say that it returns the maximum size that this particular string_ref object can hold without reassignment.
I'll think about this.
+ int compare(basic_string_ref x) const { + int cmp = std::memcmp ( ptr_, x.ptr_, std::min(len_, x.len_)); + return cmp != 0 ? cmp : ( len_ == x.len_ ? 0 : len_ < x.len_ ? -1 : 1 ); + } + + bool starts_with(charT c) const { return !empty() && front() == c; } + bool starts_with(basic_string_ref x) const { + return len_ >= x.len_ && std::memcmp ( ptr_, x.ptr_, x.len_ ) == 0; + } + + bool ends_with(charT c) const { return !empty() && back() == c; } + bool ends_with(basic_string_ref x) const { + return len_ >= x.len_ && std::memcmp ( ptr_ + len_ - x.len_, x.ptr_, x.len_ ) == 0; + } +
You can only use memcmp for char. You have to use traits::compare here.
Fixed.
+// Have to use traits here + size_type find(basic_string_ref s) const { + const_iterator iter = std::find_if ( this->cbegin (), this->cend (), + s.cbegin (), s.cend (), traits::eq ); + return iter = this->cend () ? npos : std::distance ( this->cbegin (), iter ); + } +
I think you mean std::search, not std::find_if.
D'oh!
+ size_type find(charT c) const { +#ifdef BOOST_NO_CXX11_LAMBDAS + const_iterator iter = std::find_if ( this->cbegin (), this->cend (), + detail::string_ref_traits_eq<charT, traits> ( c )); +#else + const_iterator iter = std::find_if ( this->cbegin (), this->cend (), + [c] ( charT val ) { return traits::eq ( c, val ); } ); +#endif + return iter == this->cend () ? npos : std::distance ( this->cbegin (), iter ); + } +
As with NULL/nulptr, I don't see the point of using C++11 lambdas if you have to #ifdef it with a C++03 implementation.
Thinking about this... -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
participants (2)
-
Marshall Clow -
Steven Watanabe