Re: [boost] Formal Review: Boost.RangeEx

Subject: Re: [boost] Formal Review: Boost.RangeEx
From: Vicente Botet (vicente.botet_at_[hidden])
Date: 2009-02-25 13:07:19

Hi,
----- Original Message ----- 
From: "Neil Groves" <neil_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Wednesday, February 25, 2009 6:29 PM
Subject: Re: [boost] Formal Review: Boost.RangeEx

> 
> Dear Giovanni,
> 
> On Wed, Feb 25, 2009 at 5:13 PM, Giovanni Piero Deretta <gpderetta_at_[hidden]
>> wrote:
> 
>> On Wed, Feb 25, 2009 at 5:32 PM, Neil Groves <neil_at_[hidden]>
>> wrote:
>> > Dear Giovanni,
>> >
>> > On Wed, Feb 25, 2009 at 4:21 PM, Giovanni Piero Deretta <
>> gpderetta_at_[hidden]
>> >> wrote:
>> >
>> >> On Wed, Feb 25, 2009 at 5:00 PM, Neil Groves <neil_at_[hidden]>
>> >> wrote:
>> >> > Dear Sir,
>> >> >
>> >> > On Wed, Feb 25, 2009 at 3:36 PM, Rogier van Dalen <rogiervd_at_[hidden]
>> >> >wrote:
>> >> >
>> >> >> I am not sure I understand the motivation for the operator| syntax.
>> Why
>> >> >> couldn't
>> >> >>        vec | boost::adaptors::reversed | boost::adaptors::unique
>> >> >> be written
>> >> >>        boost::adaptors::unique (boost::adaptors::reversed (vec))
>> >> >> ? I think this contrast would be fairer than what the documentation
>> >> >> currently has. Is there a reason why it is not possible to provide
>> both
>> >> >> syntaxes? The pipe syntax is nice, but sort of restricts operations
>> to 1
>> >> >> range in, 1 range out. Relatedly:
>> >> >>
>> >> >
>> >> > In the adaptors section the preference for operator| is documented. It
>> >> > simply chains better in my opinion.
>> >> >
>> >> > I do provide both alternatives, for example
>> >> >
>> >> > boost::make_uniqued_range(rng) is equivalent to rng | uniqued
>> >> >
>> >>
>> >> IMHO the make_... syntax for range adaptors is horrible (i don't even
>> >> like the past tense, why not just unique?).
>> >> But why use two different names in the first place? Why not make
>> >>
>> >
>> > I simply dislike lots of overloading particularly where the semantics are
>> > different. Perhaps the semantics aren't that different.
>>
>> IMHO they are  the same thing. I think of operator() in  ' a | b' as
>> 'b(a)', i.e as a generic apply operator.
>>
> 
> Yes, I think I might have over-emphasised the difference. Would you prefer
> the function overload to be in the boost::adaptors namespace, the boost
> namespace or something else?
>>
>> >I had considered
>> > creating a range adaptor to be highly different to applying an algorithm,
>> > perhaps I over-emphasised this distinction when making the decision.
>>
>> FWIW, I have code like this:
>>
>> total = ( r | filter(_r, f) | map(_r, m) | accumulate(_r, zero, a)  );
>>
>> i.e. I don't have a strong distinction between adaptor and algorithms.
>>
> 
> Yes, I'm sold!
 
I agree, the introduction of the parameter (_r) makes the library homogeneus. An adaptor is a functor with a placeholder for the input parameter.

>> <snip>
>> >>
>> >> 'some_functor(range) ' eqivalent to 'range | some_functor'
>> >>
>> >> (i would actually go for the syntax 'range | some_functor(_r)', where
>> >> _r is a placeholder for the range; This is usefull for adaptors that
>> >> actually have other parameters in addition to the range itself)
>> >>
>> >
>> > I would always prefer the '|' syntax to any normal function invocation
>> > however it is spelled. In my opinion, it combines much more elegantly.
>> >
>>
>> I use the '|' syntax a lot when writing long pipelines because it
>> avoids long nested expressions (it also reads better left to right).
>> But when I need a simple map or filter before passing a range to an
>> algorithm I usually just go for the function call variant.
>>
>> Also, if you have a range of ranges and want to map the nested ranges,
>> having 'transformed' as a function object becomes very useful:
>>
>> range_of_ranges | transformed(transformed(f)));
>>
> (and BTW, what about  renaming 'transformed' -> 'map' and 'filtered'
>> -> 'filter')
>>
> 
> I don't think the suggestion to change transformed is a good idea.
> transformed is more obviously related to transform, and map is already a
> standard container.

What about simply 'transform'?

> filtered to filter doesn't sound bad, not does uniqued to unique, as long as
> it doesn't clash with other names. Perhaps this suggests that they should be
> kept in the boost::adaptors namespace.

If you put then in a specific namespace you avoid clashing. If you put them in namespace boost, you need to use concepts or (enable_if). The library contains already some functions at the boost level as begin, end and nobody complains. The algorith library use three level namespace boost::algorithm::sequence for for_each_if. So the user needs 

boost::algorithm::sequence::for_each_if(range, f,cnd);

In other words, should the transform function in Boost.Fusion and in Boost.Range and in Boost.Algorithm, and .. live in different namespaces?

IMHO, Not. All these names must reside in the same namespace and the library must allows to make partial specialization of these functions. Waiting for partial function template specialization in Boost.Interthreads I have used the following workarround:

namespace boost {

    namespace result_of {
        template <typename ACT>
        struct interrupt {
            typedef void type;
        };
    }

    namespace partial_specialization_workaround {
        template< typename ACT >
        struct interrupt {
            static typename result_of::template interrupt<ACT>::type apply( ACT& act ) {
                return act.interrupt();
            }
        };
    }

    template <typename ACT>
    typename boost::enable_if<interthreads::has_interrupt_member<ACT>,
        void
    >::type
    interrupt(ACT& act) {
        return partial_specialization_workaround::interrupt<ACT>::apply(act);
    }
}   // namespace boost

The user will just do boost::interrupt(act).

If another boost library want to overload  interrupt(T) for other types that do not provide a member interrupt function, the developer needs just to 

    template <typename T>
    typename boost::enable_if<a_specific_condition<T>,
        void
    >::type
    interrupt(T& act) {   
    // ...
   }

Of course this will make the compiler to take more time, but at the end If I have an algorithm that works with let me say boost::interthreads:interrupt and boost::intertoto::interrupt how can I make this algorithm generic? 

Best,
Vicente