Tweak `make_view(iterator)` spec to match implementation

Issue #186 resolved
Dan Bonachea created an issue

The current spec for [Writer]::write_sequence has two overloads:

template <typename IterType>
std::size_t [Writer]::write_sequence(IterType begin , IterType end);
template <typename IterType>
std::size_t [Writer]::write_sequence(IterType begin , IterType end,
                                     std::size_t num_items);

However the make_view iterator interface uses a defaulted argument:

template <typename IterType>
view<typename std::iterator_traits <IterType>::value_type , IterType >
make_view(IterType begin , IterType end, 
          typename std::iterator_traits <IterType>::difference_type
              size = std::distance(begin , end));

These are both solving a similar interface problem, and AFAIK there's no good rationale for the divergence in these iterator APIs. Calls to either interface look the same in most caller code, this difference mostly affects the implementation.

IMO the former interface (two overloads) is strictly superior, because it gives the implementation the flexibility to choose the best algorithm when the user declines to provide a length. In particular, specifying a defaulted std::distance() call for that situation may incur the cost of a linear traversal before entry to the function, whereas the implementation may be able to do something smarter that performs any needed length discovery and traversal in a single pass. Further, the std::distance() approach forces a defaulted "use" of the iterators, which makes it an inappropriate design for interfaces accepting a single-pass InputIterator (eg team::create explicitly allows single-pass iterators, and I suspect [Writer]::write_sequence also meets that contract although the spec does not currently require it).

Furthermore, the make_view spec does not match the current implementation, which instead uses the two overload approach :

  template<typename Iter,
           typename T = typename std::iterator_traits<Iter>::value_type, /**/>
  view<T,Iter> make_view(Iter begin, Iter end);

  template<typename Iter,
           typename T = typename std::iterator_traits<Iter>::value_type>
  constexpr view<T,Iter> make_view(Iter begin, Iter end, std::size_t n);

This was briefly discussed on the 2021-09-13 call, and I think we have a agreement that the make_view spec should change to match the implementation and the pattern set by [Writer]::write_sequence.

On a related detail, I prefer the simplicity of specifying the size argument to be size_t, unless someone can provide a compelling reason why the signed type std::iterator_traits <IterType>::difference_type is superior in this situation. AFAIK it just degrades understanding, introduces the possibility for erroneous negative inputs, and adds no relevant benefit.

Comments (1)

  1. Log in to comment