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