[iterator] [property_map] Enhancement proposals
Dear boost, First of all, let me quickly introduce myself as this is my first post here: I am a 20 years old programming enthusiast with interest for metaprogramming, functional programming and library design. I also have a strong will to contribute to open source projects since I love the philosophy behind it and it is a great way to develop new skills and to work with talented people. The object of this message is to collect feedback on several enhancement proposals I have prepared for two existing libraries: [boost.iterator] [boost.iterator.accessor_iterator] The accessor_iterator is an iterator adaptor that returns a reference to a member of its pointee when dereferenced. I see it as another step in the direction of easier iterator composition, which has helped me writting less code in the past. [boost.iterator.chained_output_iterator] The chained_output_iterator is an iterator adaptor allowing to apply a functional transformation to a value before outputing it into another output iterator. It is much like a function_output_iterator that would forward its result to another output iterator. It allows for cool stuff like creating a pipeline of operations without having to create temporary storage. [boost.property_map] [boost.property_map.accessor_property_map] The accessor_property_map is a property map returning a reference to a member of the key it is given. It is much like the accessor_iterator for property maps. [boost.property_map.chained_property_map] The chained_property_map is a tool for property map composition. It takes two property maps and uses a value in the first one as a key in the second one to perform its put, get, and operator[] operations. [boost.property_map.property_traits] I feel like the boost/property_map/property_map.hpp header is a little bit bloated and not quite as modular as it could be. For now, I propose that we move the code dealing with property_traits into the boost/property_map/property_traits.hpp header and that we provide metafunctions to check for adherence to property map types. By providing an include in the boost/property_map/property_map.hpp header, backward compatibility is maintained and we simply increase modularity. All the code is available on github: https://github.com/ldionne/boost-submissions To see how each class can be used, you may look at the unit tests for the corresponding class. If anything is accepted, I will be pleased to write documentation and examples for the concerned feature and to integrate it into the existing documentation. Looking forward to your feedback, Louis Dionne
[Speaking as one of the maintainers of the Iterator library only...] On Sat, Sep 29, 2012 at 9:29 AM, Louis Dionne <louis.dionne92@gmail.com>wrote:
Dear boost,
First of all, let me quickly introduce myself as this is my first post here: I am a 20 years old programming enthusiast with interest for metaprogramming, functional programming and library design. I also have a strong will to contribute to open source projects since I love the philosophy behind it and it is a great way to develop new skills and to work with talented people.
Welcome!
The object of this message is to collect feedback on several enhancement proposals I have prepared for two existing libraries:
[boost.iterator] [boost.iterator.accessor_iterator] The accessor_iterator is an iterator adaptor that returns a reference to a member of its pointee when dereferenced. I see it as another step in the direction of easier iterator composition, which has helped me writting less code in the past.
[boost.iterator.chained_output_iterator] The chained_output_iterator is an iterator adaptor allowing to apply a functional transformation to a value before outputing it into another output iterator. It is much like a function_output_iterator that would forward its result to another output iterator. It allows for cool stuff like creating a pipeline of operations without having to create temporary storage.
[...] Both of these sound like useful additions; I think it makes more sense to build these on top of transform_iterator and function_output_iterator directly in some fashion. In other words, I can see 3 ways of effecting the above (and I'm not sure which you've chosen, as I haven't looked at your code submission yet): create a new iterator independent of existing iterators; use iterator_adaptor to adapt an existing iterator; or provide a, e.g., a make_access_iterator function, which returns, e.g., a transform_iterator< mem_fn_t< PtrToMemFn >, BaseIterator > (so no actual new iterator types are provided, just convenience functions). I don't really see a point in doing the first, and the last should require the least amount of additional code. I'll take a glance at your code this weekend and likely have more targeted comments. All the code is available on github:
https://github.com/ldionne/boost-submissions
To see how each class can be used, you may look at the unit tests for the corresponding class.
If anything is accepted, I will be pleased to write documentation and examples for the concerned feature and to integrate it into the existing documentation.
Always a plus :) - Jeff
On Sat, Sep 29, 2012 at 9:29 AM, Louis Dionne <louis.dionne92@gmail.com>wrote:
Dear boost,
[...]
The object of this message is to collect feedback on several enhancement proposals I have prepared for two existing libraries:
[boost.iterator] [boost.iterator.accessor_iterator] The accessor_iterator is an iterator adaptor that returns a reference to a member of its pointee when dereferenced. I see it as another step in the direction of easier iterator composition, which has helped me writting less code in the past.
Regarding accessor_iterator: The implementation is very short, but it still seems to have minimal if any advantage to just using a transform_iterator directly composed with a function object wrapping the pointer-to-member (either at runtime or compile-time [1]). Also, seems you're lacking a convenient make_accessor_iterator (which, of course, would probably have to be a macro, as described in [1]).
[boost.iterator.chained_output_iterator] The chained_output_iterator is an iterator adaptor allowing to apply a functional transformation to a value before outputing it into another output iterator. It is much like a function_output_iterator that would forward its result to another output iterator. It allows for cool stuff like creating a pipeline of operations without having to create temporary storage.
This could be useful, but there's a few things I need to understand about your implementation first. - Using is_convertible to determine iterator convertibility is, strictly speaking, too loose, as it allows derived* -> base* conversions that are undesirable if sizeof( derived ) > sizeof( base ). At some point I probably need to fish through the Iterator library and change any such uses myself... - I'm wondering why the increment of the wrapped iterator happens inside proxy::operator= rather than chained_output_iterator::operator++...? Also, typically operator* is a const member function, and moving the advancement of the wrapped iterator into chained_output_iterator::operator++ would allow that. I might be missing something though. [...]
All the code is available on github: https://github.com/ldionne/boost-submissions
To see how each class can be used, you may look at the unit tests for the corresponding class.
If anything is accepted, I will be pleased to write documentation and examples for the concerned feature and to integrate it into the existing documentation.
Looking forward to your feedback,
Louis Dionne
- Jeff [1] http://www.boost.org/doc/libs/1_51_0/libs/function_types/example/fast_mem_fn...
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
Regarding accessor_iterator: The implementation is very short, but it still seems to have minimal if any advantage to just using a transform_iterator directly composed with a function object wrapping the pointer-to-member (either at runtime or compile-time [1]). Also, seems you're lacking a convenient make_accessor_iterator (which, of course, would probably have to be a macro, as described in [1]).
Regarding the make_accessor_iterator: I did not provide such a function because I did not know how to implement it. After reading the link, I indeed see how it could be done. I will do it if the class is accepted. Another possibility is to set the pointer to member at runtime, which then makes the implementation of make_accessor_iterator trivial. You can see this alternative implementation on the dynamic_accessor_iterator branch of the git repository. Regarding the usefulness of the class vs using transform iterator: I too believe the same thing can be achieved using a transform_iterator. It really depends on whether you think creating a function object is a pain. It would be useful to know what others think of it to know whether there is a need in the community.
This could be useful, but there's a few things I need to understand about your implementation first. - Using is_convertible to determine iterator convertibility is, strictly speaking, too loose, as it allows derived* -> base* conversions that are undesirable if sizeof( derived ) > sizeof( base ). At some point I probably need to fish through the Iterator library and change any such uses myself...
Given the 3 places where I use it currently: template <typename OutIter> explicit chained_output_iterator(Function const&, OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0); template <typename OutIter> chained_output_iterator(OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0); template <typename Func, typename OutIter> chained_output_iterator(chained_output_iterator<Func, OutIter> const&, typename enable_if_convertible<Func, Function>::type* =0, typename enable_if_convertible<OutIter, OutputIterator>::type* =0); I take it that you are referring to the first two copy constructors being too loose on the `OutIter' type. Would the following be alright, given that OutputIterator is the type of the wrapped iterator? explicit chained_output_iterator(Function const&, OutputIterator const&); chained_output_iterator(OutputIterator const&);
- I'm wondering why the increment of the wrapped iterator happens inside proxy::operator= rather than chained_output_iterator::operator++...? Also,
You are right, this is my mistake and it's fixed on master now. Since this change would make the implementation with function_output_iterator more complicated than the `from scratch' implementation, I believe function_output_iterator should not be used. Note: The implementation using function_output_iterator comes from a suggestion in your first reply and is available on the using_function_output branch of the repository.
typically operator* is a const member function, and moving the advancement of the wrapped iterator into chained_output_iterator::operator++ would allow that. I might be missing something though.
In order to make the chained_output_iterator::operator* const, we would need to make the proxy hold copies or const references to the wrapped iterator and function. However, using const references would add the following restrictions: - the wrapped iterator's operator* must be const - the function object's operator() must be const For these reasons, I believe the proxy should either store copies or we should give up making chained_output_iterator::operator* const. I'll wait for your reply before I change that. Louis Dionne
On Mon, Oct 8, 2012 at 9:11 AM, Louis Dionne <louis.dionne92@gmail.com>wrote:
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
Regarding accessor_iterator: The implementation is very short, but it still seems to have minimal if any advantage to just using a transform_iterator directly composed with a function object wrapping the pointer-to-member (either at runtime or compile-time [1]). Also, seems you're lacking a convenient make_accessor_iterator (which, of course, would probably have to be a macro, as described in [1]).
Regarding the make_accessor_iterator: I did not provide such a function because I did not know how to implement it. After reading the link, I indeed see how it could be done. I will do it if the class is accepted.
Yeah, it is tricky, and the referenced solution is pretty clever :)
Another possibility is to set the pointer to member at runtime, which then makes the implementation of make_accessor_iterator trivial. You can see this alternative implementation on the dynamic_accessor_iterator branch of the git repository.
Hopefully you'll excuse me for making an educated guess on what the implementation would look like.
Regarding the usefulness of the class vs using transform iterator: I too believe the same thing can be achieved using a transform_iterator. It really depends on whether you think creating a function object is a pain. It would be useful to know what others think of it to know whether there is a need in the community.
It's just unclear to me where one should draw the line as far as what iterators should be provided by Boost.Iterator out of the box. Neither your original accessor_iterator nor dynamic_accessor_iterator cover what I would imagine to be the equally common case of applying a nullary member function to an iterator's value. So I can easily imagine 6 different variants of your accessor_iterator: { static, dynamic } x { member data pointer, non-const member function pointer, const member function pointer }. If the community pushes for including these variants directly in Boost.Iterator, that's fine, but I'm disinclined to do so otherwise given the simplicity of combining transform_iterator with the appropriate function object. Further, I don't consider the creation and use of the appropriate function objects to be a pain, given the present solutions within Boost.
your implementation first. - Using is_convertible to determine iterator convertibility is, strictly speaking, too loose, as it allows derived* -> base* conversions that are undesirable if sizeof( derived ) > sizeof( base ). At some point I
This could be useful, but there's a few things I need to understand about probably
need to fish through the Iterator library and change any such uses myself...
Given the 3 places where I use it currently:
template <typename OutIter> explicit chained_output_iterator(Function const&, OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
template <typename OutIter> chained_output_iterator(OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
template <typename Func, typename OutIter> chained_output_iterator(chained_output_iterator<Func, OutIter> const&, typename enable_if_convertible<Func, Function>::type* =0, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
I take it that you are referring to the first two copy constructors being too loose on the `OutIter' type. Would the following be alright, given that OutputIterator is the type of the wrapped iterator?
explicit chained_output_iterator(Function const&, OutputIterator const&); chained_output_iterator(OutputIterator const&);
Sorry, I had another (albeit related) concern in mind. Yes, I think it's simpler to just make the constructor parameters the wrapped iterator type directly. The instances where one needs to be a bit careful are converting constructors from one chained_output_iterator to another.
- I'm wondering why the increment of the wrapped iterator happens inside proxy::operator= rather than chained_output_iterator::operator++...? Also,
You are right, this is my mistake and it's fixed on master now.
Since this change would make the implementation with function_output_iterator more complicated than the `from scratch' implementation, I believe function_output_iterator should not be used.
Note: The implementation using function_output_iterator comes from a suggestion in your first reply and is available on the using_function_output branch of the repository.
Hmmm, yeah I don't recall anything involving function_output_iterator, I must've missed something...
typically operator* is a const member function, and moving the advancement of the wrapped iterator into chained_output_iterator::operator++ would allow that. I might be missing something though.
In order to make the chained_output_iterator::operator* const, we would need to make the proxy hold copies or const references to the wrapped iterator and function. However, using const references would add the following restrictions: - the wrapped iterator's operator* must be const
The typical case...
- the function object's operator() must be const
Again, the typical case. As a general rule of thumb, you have to be careful with state-mutating function objects immediately held within iterators, but I guess strictly incrementable iterators (as chained_output_iterator is) have more predictable access patterns so it's less of a concern.
For these reasons, I believe the proxy should either store copies or we should give up making chained_output_iterator::operator* const. I'll wait for your reply before I change that.
You can always provide the best of both by providing both const and non-const overloads of operator* (and likewise "const" and "non-const" proxy classes). I think I see chained_function_output_iterator as a viable addition to Boost.Iterator; accessor_iterator, I'm yet to be convinced. Nothing personal :) - Jeff
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
It's just unclear to me where one should draw the line as far as what iterators should be provided by Boost.Iterator out of the box. Neither your original accessor_iterator nor dynamic_accessor_iterator cover what I would imagine to be the equally common case of applying a nullary member function to an iterator's value. So I can easily imagine 6 different variants of your accessor_iterator: { static, dynamic } x { member data pointer, non-const member function pointer, const member function pointer }. If the [...]
I had not thought about it that way initially but I agree with you. So let's forget about the accessor_iterator. Since the code won't go anywhere, it can always be added later if the community asks for it.
- Using is_convertible to determine iterator convertibility is, strictly speaking, too loose, as it allows derived* -> base* conversions that are undesirable if sizeof( derived ) > sizeof( base ). At some point I probably need to fish through the Iterator library and change any such uses myself...
Given the 3 places where I use it currently:
template <typename OutIter> explicit chained_output_iterator(Function const&, OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
template <typename OutIter> chained_output_iterator(OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
template <typename Func, typename OutIter> chained_output_iterator(chained_output_iterator<Func, OutIter> const&, typename enable_if_convertible<Func, Function>::type* =0, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
I take it that you are referring to the first two copy constructors being too loose on the `OutIter' type. Would the following be alright, given that OutputIterator is the type of the wrapped iterator?
explicit chained_output_iterator(Function const&,OutputIterator const&); chained_output_iterator(OutputIterator const&);
Sorry, I had another (albeit related) concern in mind. Yes, I think it's simpler to just make the constructor parameters the wrapped iterator type directly.
Accepting any iterator that is convertible to the wrapped iterator is nice if it is not dangerous because it allows to do this: typedef chained_output_iterator<Functor1, chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>>> OutIter; // Then, if Functor1 and Functor2 are default constructible: std::vector<T> vec; OutIter result(std::back_inserter(vec)); If the constructor only takes an instance of the wrapped iterator, the following must be done, which could be annoying if one creates a long chain: typedef chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>> Chain; typedef chained_output_iterator<Functor1, Chain> OutIter; std::vector<T> vec; OutIter result(Chain(std::back_inserter(vec))); So I would advocate in favor of keeping the initial implementation for the first two constructors, unless I am missing something.
The instances where one needs to be a bit careful are converting constructors from one chained_output_iterator to another.
I don't see how this could be a problem. I don't understand in which circumstances an undesirable derived* -> base* conversion could happen. In all cases, I'll stick to a regular copy constructor unless there is a better solution.
You can always provide the best of both by providing both const and non-const overloads of operator* (and likewise "const" and "non-const" proxy classes).
Good idea, I just did that.
I think I see chained_function_output_iterator as a viable addition to Boost.Iterator; accessor_iterator, I'm yet to be convinced. Nothing personal :)
I'm glad to read that, and I agree with your for the accessor_iterator. After the last questions are settled for the chained_output_iterator, I will write more unit tests, examples and documentation for the class. Do you want me to make everything available on GitHub so you can add it to the library, or is there another preferred method? Louis Dionne
On Tue, Oct 9, 2012 at 4:37 PM, Louis Dionne <louis.dionne92@gmail.com>wrote:
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
It's just unclear to me where one should draw the line as far as what iterators should be provided by Boost.Iterator out of the box. Neither your original accessor_iterator nor dynamic_accessor_iterator cover what I would imagine to be the equally common case of applying a nullary member function to an iterator's value. So I can easily imagine 6 different variants of your accessor_iterator: { static, dynamic } x { member data pointer, non-const member function pointer, const member function pointer }. If the [...]
I had not thought about it that way initially but I agree with you. So let's forget about the accessor_iterator. Since the code won't go anywhere, it can always be added later if the community asks for it.
Deal.
- Using is_convertible to determine iterator convertibility is, strictly speaking, too loose, as it allows derived* -> base* conversions that are undesirable if sizeof( derived ) > sizeof( base ). At some point I probably need to fish through the Iterator library and change any such uses myself...
Given the 3 places where I use it currently:
template <typename OutIter> explicit chained_output_iterator(Function const&, OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
template <typename OutIter> chained_output_iterator(OutIter const&, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
template <typename Func, typename OutIter> chained_output_iterator(chained_output_iterator<Func, OutIter> const&, typename enable_if_convertible<Func, Function>::type* =0, typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
I take it that you are referring to the first two copy constructors being too loose on the `OutIter' type. Would the following be alright, given that OutputIterator is the type of the wrapped iterator?
explicit chained_output_iterator(Function const&,OutputIterator const&); chained_output_iterator(OutputIterator const&);
Sorry, I had another (albeit related) concern in mind. Yes, I think it's simpler to just make the constructor parameters the wrapped iterator type directly.
Accepting any iterator that is convertible to the wrapped iterator is nice if it is not dangerous because it allows to do this:
typedef chained_output_iterator<Functor1, chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>>> OutIter; // Then, if Functor1 and Functor2 are default constructible: std::vector<T> vec; OutIter result(std::back_inserter(vec));
If the constructor only takes an instance of the wrapped iterator, the following must be done, which could be annoying if one creates a long chain:
typedef chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>> Chain; typedef chained_output_iterator<Functor1, Chain> OutIter; std::vector<T> vec; OutIter result(Chain(std::back_inserter(vec)));
So I would advocate in favor of keeping the initial implementation for the first two constructors, unless I am missing something.
Wouldn't the first block of code above not compile regardless of whether the unary constructor is a template constrained by enable_if< is_convertible > or takes the wrapped iterator directly? The result of std::back_inserter(vec) is not convertible to the intermediate chained_output_iterator in either case. Anyway, I think the typical uses would involve type deduction via make_chained_output_iterator.
The instances where one needs to be a bit careful are converting
constructors from one chained_output_iterator to another.
I don't see how this could be a problem. I don't understand in which circumstances an undesirable derived* -> base* conversion could happen. In all cases, I'll stick to a regular copy constructor unless there is a better solution.
If you had the following chained_output_iterator constructor: template< class G, class J > chained_output_iterator(chained_output_iterator<G,J>, typename enable_if_c< mpl::and< is_convertible<H,G>, is_convertible<J,I>
::value >::type * = 0);
Oops, now chained_output_iterator< F, derived_t * > is convertible to chained_output_iterator< F, base_t * >. is_convertible<J,I> is too loose (admittedly, there are many iterator contexts where you simply cannot prevent the derived_t * -> base_t * conversion, but if you can prevent it, you should). Presently, there is no such converting constructor, and I don't yet see a need for one.
You can always provide the best of both by providing both const and
non-const overloads of operator* (and likewise "const" and "non-const" proxy classes).
Good idea, I just did that.
Ordinarily the reference typedef should be the result of operator*() const, but I guess for output iterators the reference typedef is pretty meaningless.
I think I see chained_function_output_iterator as a viable addition to Boost.Iterator; accessor_iterator, I'm yet to be convinced. Nothing personal :)
I'm glad to read that, and I agree with your for the accessor_iterator. After the last questions are settled for the chained_output_iterator, I will write more unit tests, examples and documentation for the class. Do you want me to make everything available on GitHub so you can add it to the library, or is there another preferred method?
Best (i.e., least work for me) would be to provide the header, the unit test, any patches to the present Boost.Iterator documentation, and a contract signed in blood that you'll support this for life. Well, okay, I'll take 3 out of 4 :) Where you make the files available doesn't matter too much, just as long as I can easily grab them when ready. I guess the official route is to create a feature enhancement request trac ticket with the files attached. Thanks, - Jeff
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
Accepting any iterator that is convertible to the wrapped iterator is nice if it is not dangerous because it allows to do this:
typedef chained_output_iterator<Functor1, chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>>> OutIter; // Then, if Functor1 and Functor2 are default constructible: std::vector<T> vec; OutIter result(std::back_inserter(vec));
If the constructor only takes an instance of the wrapped iterator, the following must be done, which could be annoying if one creates a long chain:
typedef chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>> Chain; typedef chained_output_iterator<Functor1, Chain> OutIter; std::vector<T> vec; OutIter result(Chain(std::back_inserter(vec)));
So I would advocate in favor of keeping the initial implementation for the first two constructors, unless I am missing something.
Wouldn't the first block of code above not compile regardless of whether the unary constructor is a template constrained by enable_if< is_convertible > or takes the wrapped iterator directly? The result of std::back_inserter(vec) is not convertible to the intermediate chained_output_iterator in either case.
Yes, the first block of code compiles. Let's say we have: typedef std::back_insert_iterator<std::vector<T> > Third; typedef chained_output_iterator<Functor2, Third> Second; typedef chained_output_iterator<Functor1, Second> First; Third is convertible to Second: template <typename Iterator> Second(Iterator const& iterator, typename enable_if_convertible<Iterator, WrappedIter>::type* =0); with [Iterator = Third] [WrappedIter = Third] (because Second::WrappedIter == Third) By the same mechanism, Second is convertible to First. Now, let's see why we can construct a First from a Third: Third third; First first(third); // This calls the template unary constructor below. First(Third const& iterator, typename enable_if_convertible<Third, Second>::type* =0) : out_(iterator) // This calls the template unary constructor below. { } Second(Third const& iterator, typename enable_if_convertible<Third, Third>::type* = 0) : out_(iterator) // This calls the constructor of Third, which is { } // std::back_insert_iterator. I hope my explanation is clear enough. There is also a unit test for this on the master branch.
If you had the following chained_output_iterator constructor:
template< class G, class J > chained_output_iterator(chained_output_iterator<G,J>, typename enable_if_c< mpl::and< is_convertible<H,G>, is_convertible<J,I>
::value >::type * = 0);
Oops, now chained_output_iterator< F, derived_t * > is convertible to chained_output_iterator< F, base_t * >. is_convertible<J,I> is too loose (admittedly, there are many iterator contexts where you simply cannot prevent the derived_t * -> base_t * conversion, but if you can prevent it, you should).
Presently, there is no such converting constructor, and I don't yet see a need for one.
Thanks for the explanation. I replaced the loose copy constructor by the one generated by the compiler.
Best (i.e., least work for me) would be to provide the header, the unit test, any patches to the present Boost.Iterator documentation, and a contract signed in blood that you'll support this for life. Well, okay, I'll take 3 out of 4 :)
Where you make the files available doesn't matter too much, just as long as I can easily grab them when ready. I guess the official route is to create a feature enhancement request trac ticket with the files attached.
Alright. I'll create a ticket when everything is ready. Since it's not getting in before at least 1.53, I'll take my time and do something good. Thanks! Louis Dionne
On Sat, Oct 13, 2012 at 4:25 PM, Louis Dionne <louis.dionne92@gmail.com>wrote:
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
Accepting any iterator that is convertible to the wrapped iterator is nice if it is not dangerous because it allows to do this:
typedef chained_output_iterator<Functor1, chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>>> OutIter; // Then, if Functor1 and Functor2 are default constructible: std::vector<T> vec; OutIter result(std::back_inserter(vec));
If the constructor only takes an instance of the wrapped iterator, the following must be done, which could be annoying if one creates a long chain:
typedef chained_output_iterator<Functor2, std::back_insert_iterator<std::vector<T>>> Chain; typedef chained_output_iterator<Functor1, Chain> OutIter; std::vector<T> vec; OutIter result(Chain(std::back_inserter(vec)));
So I would advocate in favor of keeping the initial implementation for the first two constructors, unless I am missing something.
Wouldn't the first block of code above not compile regardless of whether the unary constructor is a template constrained by enable_if< is_convertible > or takes the wrapped iterator directly? The result of std::back_inserter(vec) is not convertible to the intermediate chained_output_iterator in either case.
Yes, the first block of code compiles. Let's say we have:
typedef std::back_insert_iterator<std::vector<T> > Third; typedef chained_output_iterator<Functor2, Third> Second; typedef chained_output_iterator<Functor1, Second> First;
Third is convertible to Second:
template <typename Iterator> Second(Iterator const& iterator, typename enable_if_convertible<Iterator, WrappedIter>::type* =0);
I seemed to remember this constructor was explicit. If it isn't, it should be. If you want convenience, I would think it sufficient for make_chained_output_iterator to take multiple function objects. Or provide some component that can conveniently compose multiple function objects... (Boost.Bind and Boost.Phoenix can do this, I think, but it could probably be specialized to reduce the syntactic weight). [...]
Best (i.e., least work for me) would be to provide the header, the unit test, any patches to the present Boost.Iterator documentation, and a contract signed in blood that you'll support this for life. Well, okay, I'll take 3 out of 4 :)
Where you make the files available doesn't matter too much, just as long as I can easily grab them when ready. I guess the official route is to create a feature enhancement request trac ticket with the files attached.
Alright. I'll create a ticket when everything is ready. Since it's not getting in before at least 1.53, I'll take my time and do something good.
Sure, sounds good. Thanks, - Jeff
Hello, This is just to request feedback on the new chained_output_iterator that was proposed a while ago as an addition to the Boost.Iterator library. I have written unit tests and documentation and submitted it on boost trac. However, the ticket is assigned to "dave" instead of you so I just wanted to make sure you were aware of it. The ticket is #7597. Also, I would like to know whether you find the mean for doing iterator composition and its implementation satisfactory (using the and_then() method). Regards, Louis Dionne P.S.: I changed the name to transform_output_iterator, which is more accurate.
On Sat, Nov 10, 2012 at 11:46 AM, Louis Dionne <louis.dionne92@gmail.com>wrote:
Hello,
This is just to request feedback on the new chained_output_iterator that was proposed a while ago as an addition to the Boost.Iterator library. I have written unit tests and documentation and submitted it on boost trac. However, the ticket is assigned to "dave" instead of you so I just wanted to make sure you were aware of it. The ticket is #7597.
Also, I would like to know whether you find the mean for doing iterator composition and its implementation satisfactory (using the and_then() method).
Regards,
Louis Dionne
P.S.: I changed the name to transform_output_iterator, which is more accurate.
Great thanks Louis. Sadly I won't be able to review it until the week after next, I think. If it starts languishing be sure to ping me :) - Jeff
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
Great thanks Louis. Sadly I won't be able to review it until the week after next, I think. If it starts languishing be sure to ping me :)
- Jeff
No problem, I just wanted to make sure. Louis
participants (2)
-
Jeffrey Lee Hellrung, Jr. -
Louis Dionne