Re: [boost] Assign V2 - first impression

Subject: Re: [boost] Assign V2 - first impression
From: Christian Holmquist (c.holmquist_at_[hidden])
Date: 2011-06-23 15:02:39

On 23 June 2011 12:06, er <er.ci.2020_at_[hidden]> wrote:

>
>  While you only state a preference over V2, it's hard to see that you are
>>> unhappy with it.
>>>
>>>  I think your library needs to do better than coming up with a slightly
>> different syntax than V1, otherwise I don't see the point of having it.
>>
>
> Indeed, but I claim more than a slightly different syntax. The fixed-arity
> stuff is quite a different syntax. The other stated goals of the design, in
> the change-log, are, amongst other things, to bring these improvements:
>
> 1- Code reuse because deque and put share the same crtp
>

Are we talking about code reuse within the library? A library isn't useful
on its merits to not have unnecessary code duplication in its
implementation.
Or did I misunderstand this point?

> 2- Code decoupling (modularity)
>
How does this benefit me as a user of the library?

> 3- Open for extension (albeit not enough, see my response to)
>
> Dito.

Do I have to argue that these objectives worthwhile?! No, so I please ask
> whether they are were met, and if not, what is lacking. PS: maybe I did not
> state 3-.
>

Yes, please argue explicitly how these improvements over V1 benefits the
user of the library.

>
> Let's assume I get paid for writing a TPS report, every day, involving 100
> unit-tests such as the one below.
>
> STL approach:
>
> std::deque<int> cont;
> const int n = 2;
> int x = log10( 1 );
> for(int i = 0; i < n; i++)
> {
>    cont.push_front( x );
> }
> x = log10( 10 );
> for(int i = 0; i < n; i++)
> {
>    cont.push_front( x );
> }
> x = log10( 100 );
> for(int i = 0; i < n; i++)
> {
>    cont.push_front( x );
> }
> x = log10( 1000 );
> for(int i = 0; i < n; i++)
> {
>   cont.push_front( x );
> }
>
> assert(
>    boost::range::equal(
>        cont,
>        csv_deque( 3, 3, 2, 2, 1, 1, 0, 0 )
>    )
> );
>
> PS: No need to argue that a nested loop would made the job easier. Your
> boss can make it as arcane as he likes and he might have as well re-arranged
> 1, 10, 100, 1000 in a different order.
>

array<int, 4> log_constants = {1, 10, 100, 1000};
vector<int> cont;
int n = 2;
foreach(i, log_constants)
  int x = log10(i);
  for(int  j = 0; j < n;++j)
  {
    cont.push_back(x);
  }

I doesn't matter, really, how the values are arranged or what kind of boss
you have. I don't know if I got the above code correctly, but it doesn't
matter. The point is that it's readable.

>
> Assign 2.0:
>
> BOOST_AUTO( _r, ( _repeat = 2 ) );
> typedef function<int(int)> f_;
> BOOST_AUTO( _d, ( _data = f_( log10 ) ) );
>
> assert(
>    boost::range::equal(
>        csv(
>            deque<int, push_front_>( _nil) % _d % _r,
>            1, 10, 100, 1000
>        ),
>        csv_deque( 3, 3, 2, 2, 1, 1, 0, 0 )
>    )
> );
>
> Does Assign (1.0-2.0) begins to add positively to your well being?
>

I'm sorry, not to me.
The STL code I can understand, and I could compact that myself quite a bit
with some useful snippets if my daily work involved this kind of code
repeatedly.
If you and I were at the same company, I could take over the STL example for
future maintenance, but the assign V2 code I personally cannot parse.

>
> Assume, further, that you have to do the same thing all over again, but you
> have to drop the function altogether, and replace push_front by push_back.
> Is it better to write a function or use Assign?
>

You lost me here. Guessing wildly, I would probably respond "write a
function" and you would say "use Assign".

> As for 2.0, specifically, do we agree about *decoupling* of features (_r,_d
> and push_front_)?
>

I don't know.  What is _r, _d and push_front_?

> The same code above requires very minor modification if used with put()
> rather than deque(), are we on the same page about *code reuse*?
>
?

>
> Finally, let's say you want something other than repeat, that does not
> already exits. You can write write a small class, that conforms to a concept
> (ConceptModifier), and invoke two mere macros, and the job is done. Do you
> agree about *open for extension*?
>
> https://svn.boost.org/svn/**boost/sandbox/assign_v2/boost/**
> assign/v2/option/modifier/**repeat.hpp<https://svn.boost.org/svn/boost/sandbox/assign_v2/boost/assign/v2/option/modifier/repeat.hpp>
>
> Please have a bit mercy and give me your updated impressions based on these
> clarifications.
>
>
>
Let's not get overly dramatic, shall we =)

I get the impression that Assign V2 mixes too many features under one hood,
and on top of that uses an unorthodox ways of expressing itself. I feel that
assignment and algorithms should not be interleaved in this way.
Skimming trough the docs and examples leaves me very puzzled. I've now spent
a few hours on this library, but I've not been able yet to figure out the
first code line from the docs

cal | do_csv_put<2>( "jan", 31, "feb", 28, "mar", 31 ),

The difference between do_csv, put, _put, csv_put in all forms are subtle to
me.
I think they all return a proxy object containing a conversion operator and
a magic for_each member function, but I could be wrong.

I would still like to a see a clear, concise and easy to read example that
shows:
* The example can't be expressed neatly with V1.
* The example can't be expressed neatly with standard tools, such as
Boost.Range + Boost.Lamba/Phoenix,

The example would need to guide me trough this process, and it needs to make
a -best effort- in using standard tools, and then consistently showing how
they fail.

- C