Lazy string_view splitter range

Question

Trying something out of my comfort zone here, and attempting to implement a D-style range around std::string_view.

Looking for any and all suggestions or improvements.

One of the things I'm not exactly happy about is having to declare difference_type, pointer and reference just to fullfill the forward_iterator concept properly, yet not being able to actually use them.

I feel that this allowing me to initialize a vector straight from the iterator pair is worth the clunkiness, but I welcome any opinion on the subject, or suggestions on how to make it better.

Thanks!

#include <iterator>
#include <string_view>

template <typename CharT>
struct SplitStringIterator {
  using difference_type = std::ptrdiff_t;
  using value_type = std::basic_string_view<CharT>;
  using pointer = std::basic_string_view<CharT>*;
  using reference = std::basic_string_view<CharT>&;
  using iterator_category = std::forward_iterator_tag;

  SplitStringIterator(value_type str, CharT delim)
      : remainder_(str), delim_(delim) {
    advance_();
  }

  SplitStringIterator& operator++() {
    advance_();
    return *this;
  }

  SplitStringIterator operator++(int) {
    SplitStringIterator other = *this;
    advance_();
    return other;
  }

  value_type operator*() const { return sub_str_; }

  bool operator==(SplitStringIterator const& rhs) const {
    return sub_str_.data() == rhs.sub_str_.data() &&
           remainder_.data() == rhs.remainder_.data() && delim_ == delim_;
  }

  bool operator!=(SplitStringIterator const& rhs) const {
    return !(*this == rhs);
  }

 private:
  value_type sub_str_;
  value_type remainder_;
  const CharT delim_;

  void advance_() {
    auto delim_pos = remainder_.find(delim_);

    if (delim_pos == value_type::npos) {
      sub_str_ = remainder_;
      remainder_ = value_type();
    } else {
      sub_str_ = value_type(&*remainder_.begin(), delim_pos);
      remainder_.remove_prefix(delim_pos + 1);
    }
  }
};

template <typename CharT>
struct SplitString {
  SplitString(std::basic_string_view<CharT> src, CharT d)
      : source(src), delim(d) {}

  std::basic_string_view<CharT> source;
  CharT delim;

  SplitStringIterator<CharT> begin() const {
    return SplitStringIterator<CharT>(source, delim);
  }

  SplitStringIterator<CharT> end() const {
    return SplitStringIterator<CharT>(std::basic_string_view<CharT>(), delim);
  }
};

// Returns a lazy string splitter.
template <typename CharT>
SplitString<CharT> split(std::basic_string_view<CharT> str, CharT delim) {
  return SplitString<CharT>(str, delim);
}

Usage example:

#include <vector>
#include <iostream>

void foo() {
  std::string data = "aa:bb:cccc";
  auto parts = split(std::string_view(data), ':');

  for(auto p : parts) {
    std::cout << p << std::endl;
  }

  //if we need random access:
  std::vector<std::string_view> parts_indexable(parts.begin(), parts.end());
}

Answer 1

Implementation

• operator==(SplitStringIterator const&) only compares pointers for equality, not lengths. This might not be intended.
• The ForwardIterator concept isn't actually fulfilled: For that, operator* has to return a reference! (And it makes sense, too: You cannot provide a multipass guarantee in case the contents get modified, i.e. a previously different CharT value gets changed to delim.)
• Why restrict delim to CharT, when it could just as easily be a std::basic_string_view<CharT>? The advance_() logic would have to be updated for the varying length, but that's the only non-type change I can see that would be necessary. (Though one could add a conversion or a special case for single CharT delimiters.)

Small caveat: If parts_indexable had a longer lifetime than data (e.g. it got returned by foo), all its contents would dangle (though this is inherent problem of std::string_view, and not of your implementation). This could be fixed by making explicit copies by changing the type of parts_indexable to std::vector<std::string>. (Not a criticism of your work, just a note for users of this iterator).

Answer 2

Naming

My preference for range would be string_splitter. For iterator, I would prefer splitter_iterator.

Semantics

The semantics look much like std::istream_iterator. As a result, since std::string_view is non-owning range and charT is usually default constructible, may be it is good to put a default constructor, so iterator based algorithms could still use it.

range_splitter():
         //value initialize everything to zero/default
{}

Default-constructed iterator would act like a sentinel value for the range.

Also, it seems that providing a reference on dereference is quite risky, since users could insert delim somewhere in the middle of the range they got. The idea is good, but I believe it could be improved to make it harder to use incorrectly. InputIterator seems much more natural for me.