Subject: Re: [boost] [Boost-commit] svn:boost r81360 - in trunk: boost/algorithm libs/algorithm/test
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2012-11-15 16:32:38
AMDG
On 11/15/2012 11:45 AM, marshall_at_[hidden] 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