8

(also see Is there a good way not to hand-write all twelve required Container functions for a custom type in C++? )


For a class such as

namespace JDanielSmith {
class C
{
    const size_t _size;
    const std::unique_ptr<int[]> _data;

public:
    C(size_t size) : _size(size), _data(new int[size]) {}

    inline const int* get() const noexcept { return _data.get(); }
    inline int* get() noexcept { return _data.get(); }

    size_t size() const noexcept { return _size; }
};
}

what is the preferred way to expose iteration? Should I write begin()/end() (and cbegin()/cend()) member functions?

const int* cbegin() const {
    return get();
}
const int* cend() const {
    return cbegin() + size();
}

or should these be non-member functions?

const int* cbegin(const C& c) {
    return c.get();
}
const int* cend(const C& c) {
    return cbegin(c) + c.size();
}

Should begin()/end() have both const and non-const overloads?

    const int* begin() const {
        return get();
    }
    int* begin() {
        return get();
    }

Are there any other things to consider? Are there tools/techniques to make this "easy to get right" and reduce the amount of boiler-plate code?


Some related questions/discussion include:

Community
  • 1
  • 1
Ðаn
  • 10,934
  • 11
  • 59
  • 95
  • 1
    There should be both; members as well as free (or consider if `std::begin` and `std::end` pairs for your case or not before adding *free* versions). Also, you should also have `begin()` and `end()` pairs. And also, member types, `iterator` and `const_iterator` instead of `const int*` or so. – Nawaz Aug 30 '16 at 15:34
  • @SteveJessop: No. I didn't *mean* that at all. I said in the general sense: if `std::begin` doesn't work for you, then you should add yours, in the same namespace, so that ADL should work. – Nawaz Aug 30 '16 at 15:37
  • 1
    @Dan: For the code you have posted, you dont have to write the *free* versions, because `std::begin` family would work just fine. – Nawaz Aug 30 '16 at 15:40
  • 1
    Related: [Should custom containers have free begin/end functions?](http://stackoverflow.com/q/17562943/819272) – TemplateRex Aug 30 '16 at 17:07

4 Answers4

6

There is a standard which describes what your class interfaces should look like if you want them to be congruent with the STL. C++ has this notion of 'concepts' which pin down the requirements for a given class to be a sufficient implementation of a concept. This almost became a language feature in c++11.

A concept you may be interested in is the Container concept. As you can see, in order to meet the requirements of the Container concept, you need begin, cbegin, end, and cend as member functions (among other things).

Since it looks like you're storing your data in an array, you might also be interested in SequenceContainer.

bfair
  • 1,101
  • 7
  • 16
  • 2
    Container has some things in it that this class currently doesn't, the big one being copying. Alternatives would be: satisfy the requirements of Boost.Range, which is "something you can iterate and hence use algorithms on"; or just do enough that range-based for loops work. – Steve Jessop Aug 30 '16 at 15:52
  • Well. Raw arrays aren't Containers. They're "congruent" with everything in ``. – Barry Aug 30 '16 at 15:52
  • @Dan Another way of doing that is to make `C` a template which allows you to specify the way the data is stored. This way you can keep the class interface as member functions which is more conventional (perhaps that's just my opinion). You can use `std::begin` with `C` if you need a `begin` that isn't a member assuming `C::begin` is reasonable. – bfair Aug 30 '16 at 16:10
  • 1
    @Dan Well, I would point back to my original answer about the Container concept and Steve Jessop's comment about the Boost.Range concept (which is more minimal and directly what you're asking about). This is what is required for conventional iteration. – bfair Aug 30 '16 at 16:28
  • Note that they had to change their definition/prerequisite of Container to include new container as `std::array`... – Jarod42 Aug 30 '16 at 18:09
2

I'll take option C.

The main problem here is that std::begin() doesn't actually work for finding non-member begin() with ADL. So the real solution is to write your own that does:

namespace details {
    using std::begin;

    template <class C>
    constexpr auto adl_begin(C& c) noexcept(noexcept(begin(c)))
        -> decltype(begin(c))
    {
        return begin(c);
    }
}

using details::adl_begin;

Now, it doesn't matter if you write your begin() as a member or non-member function, just use adl_begin(x) everywhere and it'll just work. As well as for both the standard containers and raw arrays. This conveniently side-steps the member vs. non-member discussion.


And yes, you should have const and non-const overloads of begin() and friends, if you want to expose const and non-const access.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • 1
    @Dan Would you rather remember to have to litter your code with `using std::begin; using std::end;`? – Barry Aug 30 '16 at 15:46
  • You also need to make that `constexpr` and `noexcept` ;-) – Kerrek SB Aug 30 '16 at 15:49
  • So why not just implement a member begin and use `std::begin` everywhere? – MikeMB Aug 30 '16 at 16:08
  • 1
    @MikeMB Because that won't work on types that have a non-member `begin()`. – Barry Aug 30 '16 at 16:18
  • 1
    See also http://stackoverflow.com/questions/17562943/should-custom-containers-have-free-begin-end-functions – TemplateRex Aug 30 '16 at 17:12
  • 1
    @TemplateRex It's weird to list `adl_begin()` as being less terse than `begin()` as a con, when being able to write `begin()` requires also writing `using std::begin;` which is net more characters? – Barry Aug 30 '16 at 18:31
  • @TemplateRex Also definitely don't understand the pollute namespace with `std::begin` comment. – Barry Aug 30 '16 at 18:32
  • I think that was because pre-C++14 auto return, you needed to write `using std::begin` at namespace scope in order for `->decltype(begin(bla)))` to work, a header including `adl_begin` would pollute the namespace – TemplateRex Aug 30 '16 at 18:45
  • @TemplateRex Not the global namespace. In my example, I'm polluting `details`, not `::` – Barry Aug 30 '16 at 18:48
  • @Barry I see, well, the Q&A is community wiki, so feel free to edit. – TemplateRex Aug 30 '16 at 19:05
  • 2
    @Barry: "*Because that won't work on types that have a non-member begin().*" But you're *writing* the container. Your container can have member `begin` if you want it to. I can understand adding non-member specializations to `std::begin/end` for a container you don't control. But if its your container, I see nothing to be gained from going the non-standard route of using member functions. – Nicol Bolas Aug 31 '16 at 16:32
1

I suggest creating both sets of functions -- member functions as well as non-member functions -- to allow for maximum flexibility.

namespace JDanielSmith {
   class C
   {
      const size_t _size;
      const std::unique_ptr<int[]> _data;

      public:
      C(size_t size) : _size(size), _data(new int[size]) {}

      inline const int* get() const { return _data.get(); }
      inline int* get() { return _data.get(); }

      size_t size() const { return _size; }

      int* begin() { return get(); }
      int* end() { return get() + _size; }
      const int* begin() const { return get(); }
      const int* end() const { return get() + _size; }
      const int* cbegin() const { return get(); }
      const int* cend() const { return get() + _size; }

   };

   int* begin(C& c) { return c.begin(); }
   int* end(C& c) { return c.end(); }
   const int* begin(C const& c) { return c.begin(); }
   const int* end(C const& c) { return c.end(); }
   const int* cbegin(C const& c) { return c.begin(); }
   const int* cend(C const& c) { return c.end(); }
}

The member functions are necessary if you want to be able to use objects of type C as arguments to std::begin, std::end, std::cbegin, and std::cend.

The non-member functions are necessary if you want to be able to use objects of type C as arguments to just begin, end, cbegin, and cend. ADL will make sure that the non-member functions will be found for such usages.

int main()
{
   JDanielSmith::C c1(10);

   {
      // Non-const member functions are found
      auto b = std::begin(c1);
      auto e = std::end(c1);
      for (int i = 0; b != e; ++b, ++i )
      {
         *b = i*10;
      }
   }

   JDanielSmith::C const& c2 = c1;
   {
      // Const member functions are found
      auto b = std::begin(c2);
      auto e = std::end(c2);
      for ( ; b != e; ++b )
      {
         std::cout << *b << std::endl;
      }
   }

   {
      // Non-member functions with const-objects as argument are found
      auto b = begin(c2);
      auto e = end(c2);
      for ( ; b != e; ++b )
      {
         std::cout << *b << std::endl;
      }
   }
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • 3
    Am I missing something? Why is everyone implementing their own non-member `begin` and `end` functions? Why don't `std::begin` and `std::end` suffice? – bfair Aug 30 '16 at 16:13
  • 1
    @Dan, it is a lot of boiler-plate code. The cost of creating such boiler-plate code is very small compared to the benefits. – R Sahu Aug 30 '16 at 16:13
  • 1
    @Dan, if you have a need to create many such containers, you might be able to use macros to reduce the amount of hand-typed code. I would use macros if I have more than two containers. – R Sahu Aug 30 '16 at 16:18
  • 2
    @BrianFairservice: http://stackoverflow.com/questions/7593086/why-use-non-member-begin-and-end-functions-in-c11 – Guvante Aug 30 '16 at 16:18
  • 2
    "The non-member functions are necessary if you want to be able to use objects of type C as arguments to just begin, end, cbegin, and cend" No you don't, the default `std::begin` will already call the member version. You only need to write the non member versions if you literally cannot add member versions, like with arrays or 3rd parties. – Mooing Duck Sep 01 '16 at 20:50
  • @MooingDuck, I agree that you don't need the non-member functions if you use `std::begin`. You need them for ADL if you use just `begin`. – R Sahu Sep 01 '16 at 20:56
  • 1
    @RSahu: Why? Wont unqualified `begin` fall back on `std::begin`, which then calls the member functions? – Mooing Duck Sep 01 '16 at 20:57
  • @MooingDuck, no. I verified that when I was formulating the answer. – R Sahu Sep 01 '16 at 20:58
  • @Guvante: Top answer says the only advantage is "Free-functions... they can be added afterwards, on a data-structure you cannot alter." which doesn't apply here – Mooing Duck Sep 01 '16 at 21:03
  • @MooingDuck, remove `using std::begin;`. – R Sahu Sep 01 '16 at 21:04
  • @MooingDuck: Templated functions can use the free functions as a simpler way to handle different data types is the point. So if you have generic code to walk a collection it works for `std::vector`, `int[]` and your custom type `T` with free function `begin(T)`, `end(T)`. – Guvante Sep 01 '16 at 22:10
  • 4
    RSahu: That's the normally recommended way to do things, I don't see a reason to remove it. @Guvante: I know the benefits of a global begin, I just don't see the benefits of a _custom_ global begin. The default looks like it works fine. – Mooing Duck Sep 01 '16 at 23:12
-2

In order to create a valid iterator, you must ensure that std::iterator_traits is valid. This means you must set the iterator category among other things.

An iterator should implement iterator(), iterator(iterator&&), iterator(iterator const&), operator==, operator !=, operator++, operator++(int), operator*, operator=, and operator->. It's also a good idea to add operator< and operator+ if you can (you can't always, e.g. linked lists.)

template <typename T>
class foo
{
public:

  using value_type = T;
  class iterator 
  { 
  public:
    using value_type = foo::value_type;
    using iterator_category = std::random_access_iterator_tag;
    // or whatever type of iterator you have...
    using pointer = value_type*;
    using reference = value_type&;
    using difference_type = std::ptrdiff_t;

    // ... 
  };

  class const_iterator 
  {
    // ... 
  };

  iterator begin() { /*...*/ }
  iterator end() { /*...*/ }

  const_iterator cbegin() const { /*...*/ }
  const_iterator cend() const { /*...*/ }

  /* ... */
};

See: http://en.cppreference.com/w/cpp/iterator/iterator_traits for more information on what you need to make a valid iterator. (Note: You also need certain properties to be a valid "container", like .size())

Ideally you should use member functions for begin and end, but it's not required... you can also overload std::begin and std::end. If you don't know how to do that, I suggest you use member functions.

You should create begin() const and end() const, but it should be an alias for cbegin(), NEVER the same as begin()!

Exaeta
  • 300
  • 1
  • 4