[bimap][multi_index][range] sub_range vs std::pair<iter, iter>
Hi, Boost.MultiIndex and Boost.Bimap implements a function named range. It is designed to integrate well with the new Boost.Range framework. The actual signature is: template< class LowerBound, class UpperBound > std::pair<iterator,iterator> range(LowerBound lower, UpperBound upper); template< class LowerBound, class UpperBound > std::pair<const_iterator, const_iterator> range(LowerBound lower, UpperBound upper) const; In bimap docs it is stated that the way to use it is: typedef bimap<string,int> bm_type; bm_type bm; //... std::pair<bm_type::left_iterator, bm_type::left_iterator>r = bm.left.range(...,...); I want to know if this is the best approach. Boost.Range defines two range classes (iterator_range and sub_range) http://www.boost.org/libs/range/doc/utility_class.html It is said that they are better abstraction for ranges that std::pair<iter,iter> 1) Do you think that the range functions should return a sub_range<map_type> instead of a std::pair<iter,iter>? The signature will be: template< class LowerBound, class UpperBound > sub_range<map_type> range(LowerBound lower, UpperBound upper); template< class LowerBound, class UpperBound > const sub_range<map_type> range(LowerBound lower, UpperBound upper) const; 2) In any case do you think that including typedef for the returned range type will make code more readable? The idea is to define: typedef -range_type- sub_range; // the name can be different typedef -const_range_type- const_sub_range; So the user code will look like this: typedef bimap<string,int> bm_type; bm_type bm; //... bm_type::left_sub_range r = bm.left.range(...,...); Best regards Matias
----- Mensaje original ----- De: Matias Capeletto <matias.capeletto@gmail.com> Fecha: Miércoles, Mayo 23, 2007 11:20 pm Asunto: [boost] [bimap][multi_index][range] sub_range vs std::pair<iter, iter> Para: "boost@lists.boost.org" <boost@lists.boost.org>
Hi,
Boost.MultiIndex and Boost.Bimap implements a function named range. It is designed to integrate well with the new Boost.Range framework.
The actual signature is:
template< class LowerBound, class UpperBound > std::pair<iterator,iterator> range( LowerBound lower, UpperBound upper); template< class LowerBound, class UpperBound > std::pair<const_iterator, const_iterator> range(LowerBound lower, UpperBound upper) const;
In bimap docs it is stated that the way to use it is:
typedef bimap<string,int> bm_type; bm_type bm; //... std::pair<bm_type::left_iterator, bm_type::left_iterator>r = bm.left.range(...,...);
I want to know if this is the best approach. Boost.Range defines two range classes (iterator_range and sub_range) http://www.boost.org/libs/range/doc/utility_class.html It is said that they are better abstraction for ranges that std::pair<iter,iter>
1) Do you think that the range functions should return a sub_range<map_type>instead of a std::pair<iter,iter>? [...]
IMHO, no, so as to avoid unasked for dependencies between libraries --both at a conceptual and #include level. Moreover, one can always convert the std::pair to a range very easily, although this conversion would be much simpler if iterator_range and sub_range provided ctors taking std::pairs of iterators --Thorsten, would you consider adding this a good idea?
2) In any case do you think that including typedef for the returned range type will make code more readable? The idea is to define:
typedef -range_type- sub_range; // the name can be different typedef -const_range_type- const_sub_range;
So the user code will look like this:
typedef bimap<string,int> bm_type; bm_type bm; //... bm_type::left_sub_range r = bm.left.range(...,...);
I am not sure. On one hand, the syntax above is obviously more concise, but on the other hand a casual reader would have to look up what sub_range is to fully understand the code, whereas the std::pair is self explanatory and moreover it's honored by the tradition of equal_range(). Just my 2c, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
On 5/23/07, "JOAQUIN LOPEZ MU?Z" <joaquin@tid.es> wrote:
----- Mensaje original -----
1) Do you think that the range functions should return a sub_range<map_type>instead of a std::pair<iter,iter>? [...]
IMHO, no, so as to avoid unasked for dependencies between libraries --both at a conceptual and #include level.
Lets see what Thorsten have to said about this. IMO in this case the dependency will not heart anybody. Looking at this code: map_type::sub_range r = m.range(...,...); for( map_type::iterator i = r.begin(), iend = r.end(); i != iend; ++i) { .... } It is IMO easier to explain than: map_type::sub_range r = m.range(...,...); for( map_type::iterator i = r.first, iend = r.second; i != iend; ++i) { .... } But I have to admit that if Boost.Range framework is used both examples are equal: map_type::sub_range r = m.range(...,...); for( map_type::iterator i = begin(r), iend = end(r); i != iend; ++i) { .... }
Moreover, one can always convert the std::pair to a range very easily, although this conversion would be much simpler if iterator_range and sub_range provided ctors taking std::pairs of iterators --Thorsten, would you consider adding this a good idea?
They are already provided. I have just being playing with them with Boost.Bimap.
2) In any case do you think that including typedef for the returned range type will make code more readable? The idea is to define:
typedef -range_type- sub_range; // the name can be different typedef -const_range_type- const_sub_range;
So the user code will look like this:
typedef bimap<string,int> bm_type; bm_type bm; //... bm_type::left_sub_range r = bm.left.range(...,...);
I am not sure. On one hand, the syntax above is obviously more concise, but on the other hand a casual reader would have to look up what sub_range is to fully understand the code, whereas the std::pair is self explanatory and moreover it's honored by the tradition of equal_range().
At least for Boost.Bimap, compare: std::pair< bm_type::map_by<name>::const_iterator, bm_type::map_by<name>::const_iterator
r = bm.by<name>().range(...,....);
with: bm_type::map_by<name>::const_sub_range r = bm.by<name>().range(...,....); Maybe "sub_range" is not the better word. IMO it is not a bad election. Best regards Matias
Matias Capeletto skrev:
On 5/23/07, "JOAQUIN LOPEZ MU?Z" <joaquin@tid.es> wrote:
----- Mensaje original -----
[snip]
Moreover, one can always convert the std::pair to a range very easily, although this conversion would be much simpler if iterator_range and sub_range provided ctors taking std::pairs of iterators --Thorsten, would you consider adding this a good idea?
They are already provided. I have just being playing with them with Boost.Bimap.
It's there implicitly because the two classes has a templated constructor that takes a Range template argument.
2) In any case do you think that including typedef for the returned range type will make code more readable? The idea is to define:
typedef -range_type- sub_range; // the name can be different typedef -const_range_type- const_sub_range;
So the user code will look like this:
typedef bimap<string,int> bm_type; bm_type bm; //... bm_type::left_sub_range r = bm.left.range(...,...); I am not sure. On one hand, the syntax above is obviously more concise, but on the other hand a casual reader would have to look up what sub_range is to fully understand the code, whereas the std::pair is self explanatory and moreover it's honored by the tradition of equal_range().
At least for Boost.Bimap, compare:
std::pair< bm_type::map_by<name>::const_iterator, bm_type::map_by<name>::const_iterator
r = bm.by<name>().range(...,....);
with:
bm_type::map_by<name>::const_sub_range r = bm.by<name>().range(...,....);
Maybe "sub_range" is not the better word. IMO it is not a bad election.
boost::sub_range< bm_type::map_by<name> > r = bm.by<name>().range(...,...); is not that bad IMO. This should work with or without dependency on Boost.Range. Without dependency there might be an extra conversion, I haven't timed code to see if it really matters. -Thorsten
On 5/24/07, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
Matias Capeletto skrev:
On 5/23/07, "JOAQUIN LOPEZ MU?Z" <joaquin@tid.es> wrote:
----- Mensaje original -----
[snip]
2) In any case do you think that including typedef for the returned range type will make code more readable? The idea is to define:
typedef -range_type- sub_range; // the name can be different typedef -const_range_type- const_sub_range;
So the user code will look like this:
typedef bimap<string,int> bm_type; bm_type bm; //... bm_type::left_sub_range r = bm.left.range(...,...); I am not sure. On one hand, the syntax above is obviously more concise, but on the other hand a casual reader would have to look up what sub_range is to fully understand the code, whereas the std::pair is self explanatory and moreover it's honored by the tradition of equal_range().
I think I will leave the range function return a pair<iter,iter> like equal_range in the end.
At least for Boost.Bimap, compare:
std::pair< bm_type::map_by<name>::const_iterator, bm_type::map_by<name>::const_iterator
r = bm.by<name>().range(...,....);
with:
bm_type::map_by<name>::const_sub_range r = bm.by<name>().range(...,....);
Maybe "sub_range" is not the better word. IMO it is not a bad election.
boost::sub_range< bm_type::map_by<name> > r = bm.by<name>().range(...,...);
is not that bad IMO. This should work with or without dependency on Boost.Range. Without dependency there might be an extra conversion, I haven't timed code to see if it really matters.
If bm is const I think we have to write: boost::sub_range< const bm_type::map_by<name> > r = bm.by<name>().range(...,...); that it is not bad anyway. But I really like the look of the following code: bm_type::map_by<name>::const_iterator_range r = bm.equal_range(...); bm_type::map_by<name>::iterator_range r = bm.range(...,...); Are you against provide these typedefs? (maybe with another name: sub_range, range, range_type) Best regards Matias
Matias Capeletto skrev:
On 5/24/07, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
If bm is const I think we have to write:
boost::sub_range< const bm_type::map_by<name> > r = bm.by<name>().range(...,...);
right.
that it is not bad anyway. But I really like the look of the following code:
bm_type::map_by<name>::const_iterator_range r = bm.equal_range(...);
bm_type::map_by<name>::iterator_range r = bm.range(...,...);
Are you against provide these typedefs?
not at all.
(maybe with another name: sub_range, range, range_type)
simply "range" and "cont_range" is more concise IMO. The relation to iterators is immplied. -Thorsten
But I really like the look of the following code:
bm_type::map_by<name>::const_iterator_range r = bm.equal_range(...);
bm_type::map_by<name>::iterator_range r = bm.range(...,...);
Are you against provide these typedefs?
not at all.
Ok, I will add them.
(maybe with another name: sub_range, range, range_type)
simply "range" and "cont_range" is more concise IMO. The relation to iterators is immplied.
Boost.MultiIndex and Boost.Bimap defines a function "range(... , ...)". We can not use the name range for the typedef... :( I vote for range_type and const_range_type ( Note that we can not change the range function signature because MultiIndex has use it for years ) Best regards Matias
Matias Capeletto wrote:
1) Do you think that the range functions should return a sub_range<map_type> instead of a std::pair<iter,iter>?
'boost::iterator_range' or 'std::pair' seems a good choice. 'boost::sub_range' propagates const-ness. 'boost::begin(x.range(...,...))' returns a "constant" iterator, because a rvalue cannot be bound to non-const-reference. In fact, I still don't know where 'boost::sub_range' is useful. Well, one thing I am a little worried about is that 'boost::iterator_range' doesn't work with 'boost::is_convertible'. Regards, -- Shunsuke Sogame
shunsuke skrev:
Matias Capeletto wrote:
1) Do you think that the range functions should return a sub_range<map_type> instead of a std::pair<iter,iter>?
'boost::iterator_range' or 'std::pair' seems a good choice. 'boost::sub_range' propagates const-ness. 'boost::begin(x.range(...,...))' returns a "constant" iterator, because a rvalue cannot be bound to non-const-reference. In fact, I still don't know where 'boost::sub_range' is useful.
Well, one thing I am a little worried about is that 'boost::iterator_range' doesn't work with 'boost::is_convertible'.
Could you explain please? Thanks -Thorsten
shunsuke skrev:
Thorsten Ottosen wrote:
Well, one thing I am a little worried about is that 'boost::iterator_range' doesn't work with 'boost::is_convertible'. Could you explain please?
is_convertible< int, iterator_range<char *> > returns "true".
ok, so it works afterall, you're just not satisfied with the answer :-) I guess this is because we can say iterator_range<...> r = ...; if( r ) { ... } What is your problem with that? -Thorsten
Thorsten Ottosen wrote:
shunsuke skrev:
Thorsten Ottosen wrote:
Well, one thing I am a little worried about is that 'boost::iterator_range' doesn't work with 'boost::is_convertible'. Could you explain please? is_convertible< int, iterator_range<char *> > returns "true".
ok, so it works afterall, you're just not satisfied with the answer :-)
I guess this is because we can say
iterator_range<...> r = ...; if( r ) { ... }
What is your problem with that?
I meant is_convertible< *AnyType*, // From iterator_range<char *> // To
returns "true". -- Shunsuke Sogame
shunsuke skrev:
Thorsten Ottosen wrote:
shunsuke skrev:
Thorsten Ottosen wrote:
Well, one thing I am a little worried about is that 'boost::iterator_range' doesn't work with 'boost::is_convertible'. Could you explain please? is_convertible< int, iterator_range<char *> > returns "true".
ok, so it works afterall, you're just not satisfied with the answer :-)
I guess this is because we can say
iterator_range<...> r = ...; if( r ) { ... }
What is your problem with that?
I meant
is_convertible< *AnyType*, // From iterator_range<char *> // To
returns "true".
I'm not sure why is_convertible<> behaves like that, I dont recall having done any thing to support that explicitly. Anyway, what is the problem? AFAICT, it happens for all class with a non-explicit templated constructor. -Thorsten
Thorsten Ottosen wrote:
I meant
is_convertible< *AnyType*, // From iterator_range<char *> // To
returns "true".
I'm not sure why is_convertible<> behaves like that, I dont recall having done any thing to support that explicitly.
Anyway, what is the problem? AFAICT, it happens for all class with a non-explicit templated constructor.
I sometimes expect the correct convertibility for enable_if_convertible. Well, what surprised me is that msvc fails to compile value_type x = iter[c]; // 'iter[c]' returns a proxy which is convertible to value_type. if value_type has such a constructor. Regards, -- Shunsuke Sogame
shunsuke skrev:
Thorsten Ottosen wrote:
I meant
is_convertible< *AnyType*, // From iterator_range<char *> // To
returns "true". I'm not sure why is_convertible<> behaves like that, I dont recall having done any thing to support that explicitly.
Anyway, what is the problem? AFAICT, it happens for all class with a non-explicit templated constructor.
I sometimes expect the correct convertibility for enable_if_convertible. Well, what surprised me is that msvc fails to compile value_type x = iter[c]; // 'iter[c]' returns a proxy which is convertible to value_type. if value_type has such a constructor.
You lost me again. There is far too little context to understand it properly. -Thorsten
Thorsten Ottosen wrote:
I sometimes expect the correct convertibility for enable_if_convertible. Well, what surprised me is that msvc fails to compile value_type x = iter[c]; // 'iter[c]' returns a proxy which is convertible to value_type. if value_type has such a constructor.
You lost me again. There is far too little context to understand it properly.
An iterator's bracket operator may return unspecified type(a.k.a. proxy) which is convertible to its value_type. It can be summarized as follows: struct A // value_type { A() { } template<class X> A(X x) { x.f(); } // (1) }; struct B // proxy { operator A() const { return A(); } // (2) }; int main() { A a = B(); } msvc cannot compile this, preferring (1). (I maybe found yet another msvc bug?) So I tend to think a CopyConstructible type should implement the convertibility correctly. Regards, -- Shunsuke Sogame
participants (4)
-
"JOAQUIN LOPEZ MU?Z" -
Matias Capeletto -
shunsuke -
Thorsten Ottosen