Support for std::begin & std::end #4

Closed
nova77 opened this Issue Nov 6, 2013 · 16 comments

Comments

Projects
None yet
4 participants

nova77 commented Nov 6, 2013

I have noted that your the container class must have a .begin() method otherwise you cannot declare the iterators. However, if you use declvar any iterable that support std::begin() (like static arrays) will also work in your framework. For instance, here's how I would declare a typedef for iterator_type:

typedef decltype(std::begin(std::declval<Iterable&>())) iterator_type;

Now the iterable can be:

  • vector
  • static array
  • map
    ..
Owner

ryanhaining commented Nov 6, 2013

This is cool, I'm working on this now, I seem to have completed enumerate already. Thanks

nova77 commented Nov 6, 2013

Great! Btw, along the same lines, you might want to specialize for the different types of iterators. For instance I don't think your slice would work with maps since their iterators are not random_access. I did an implementation on my own (that's where I learned about that declval trick) and specialized the iterator "jump" depending on the type: for random access it's just += step, for bidirectionals move until hitting end() or the slice limit.

Oh, and last but not least: awesome code! :) Have you already posted it on reddit/r/cpp?

Owner

ryanhaining commented Nov 6, 2013

I'll have to look into slice (though I didn't write that one), but that's a good point thanks for pointing it out. A friend of mine posted this on reedit/cpp about a month ago, here: http://www.reddit.com/r/cpp/comments/1nhulu/python_itertools_and_builtin_iteration_functions/
If you mean you implemented the specialization on this, then we'd happily look at any pull requests. Thanks a lot for the interest

nova77 commented Nov 6, 2013

Nevermind about reddit: I just saw the post.

nova77 commented Nov 6, 2013

If you mean you implemented the specialization on this, then we'd happily look at any pull requests. Thanks a lot for the interest

Mine is a bit different, but I might add a modified version if I find the time this weekend :)

Owner

ryanhaining commented Nov 6, 2013

Ok cool, hopefully I'll have these updates done by then as well.

Collaborator

aaronjosephs commented Nov 6, 2013

So 2 things.

  1. About halfway through the project I read that std::begin is now the preferred way to get an iterator, this should be relatively easy to fix though possibly even with just a regex.
  2. As for the map thing when I was cleaning up my code I noticed that some functions placed more requirements than others on the iterators. The ideal solution would be to use SFINAE to detect for things like random access and the appearance of certain methods, this somewhat tricky to do and I have a feeling that it would require some duplication of code. The good thing is that I think it would be needed for too many that only really need ++ != and *.

I think I'm going to open up a separate issue for this as it is significantly more complex than just adding std::begin everywhere

nova77 commented Nov 6, 2013

Yes, I agree. I would also replace Container with Iterable which better conveys the meaning of the data you're iterating on.

Collaborator

aaronjosephs commented Nov 6, 2013

I started updating for std::begin and std::end and noticed that for std::cbegin won't exist until c++14, I guess even the c++ standards committee forgets things, so I'm wondering if either of you had an opinion on this.
EDIT: same problem with rbegin

Owner

ryanhaining commented Nov 6, 2013

rbegin and rend can't really be relied on since the iteration protocol only requires a begin and end for iterable objects. For const objects, std::begin should return a const iterator anyway. see: testenumerate.hpp where I was able to successfully use a const string

nova77 commented Nov 6, 2013

Well, given that you don't force constness on the template argument, I just assume std::begin will pick up the correct one. In my tests with it I tried to iterate on both const variables and not (including references) and it worked.

Owner

ryanhaining commented Nov 6, 2013

We shouldn't be using cbegin cause we want the user to be able to modify the iterable through the loop. As he said, std::begin will pick const iterator if it has to

nova77 commented Nov 6, 2013

rbegin and rend can't really be relied on since the iteration protocol only requires a begin and end for iterable objects.

True, but it should be possible to "deduce" them as long as the iterator is bidirectional or random access. See this stackoverflow answer: http://stackoverflow.com/a/14468490/89640

Owner

ryanhaining commented Nov 7, 2013

The only ones that would want them would be the ones that already assume a .rbegin() or .rend() so that would be better. AFAIK slice needs random access, sorted needs an iterable that is non-exhaustible, and reverse needs the .rbegin()/.rend().
Part of my goal has been to require the least from Iterables as possible, only assuming just begin end increment, dereference, and !=, but I might have to start rethinking that.
and get rid of the boost dependency in zip_longest, but that's another story.

Contributor

n00btime commented Nov 7, 2013

Hi guys,

With regards to the SFINAE to select for different iterator "concepts", you can use std::advance(). In fact, using the iterator manipulation functions defined in <iterator> would make your classes more flexible. The others are distance, next, and prev.

http://en.cppreference.com/w/cpp/iterator

Be careful that you don't pass a negative index to advance, because then the iterators have to be BidirectionalIterators.

Owner

ryanhaining commented Nov 14, 2013

everything on the iterables has been modified to use std::begin and std::end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment