Re: [boost] Re: Formal Review: Assignment Library

From: Vladimir Prus (ghost_at_[hidden])
Date: 2004-04-08 06:32:06

Hi Thorsten,

> > to initialize both using the assign library, e.g
> >
> >    Email e;
> >    e.add_to("Winnie-the-Pooh")("Ia");
> >    e.add_cc("Story writer");
> >
> > The problem is that I can have only one 'make_insertion' for the 'Email'
> > class. Exposing the real containers used for storing addresses is not
> > good idea.
>
> I not sure why a constructor taking both a 'to' and a 'cc' cannot work for
> you
> in this case. 

Because there's separate list of several 'to' addresses and a separate list of 
'cc' addresses.  Note that I don't have a need for 'Email' class yet, it's 
just for exposition.

> But I see that the requirement of having more than "insert"
> choice might be useful.

Okay.

> So you would like something like
>
> Email e = insert_with< &Email::add_cc >( e )( ... );
>
> ? Or perhaps
>
> Email e = insert_with( bind( e, &Email::add_cc ) )(...);

Right. Though for implementing 'add_to' method inside 'Email' class it will be 
necessary make insert_assigner have template parameter for a functor type. 
For example:

    insert_assigner_with_functor< function< void (string) > >
    add_cc(const string& s)
    {
        insert_assigner_with_functor< function< void(string> > > r(
              bind(this, &Email::add_cc));

        r(s);
        return r; 
    }

> > The operator<< declared in the namespace will hide the one from "using
> > namespace", and unless user is aware about this problem he might spend a
>
> lot
>
> > of time trying to understand why the right operator<< is not selected. I
>
> know
>
> > it costed me about 1 hour some time ago.
>
> Thanks for pointing this out. I'll change 'using namespace' to a namespace
> alias
> and make a warning about this behavior in a footnote.

In fact,

  using assignment::operator<<

does not have this problem, though it's cumbersome.

> > 3. While the += operator is fine with me, I don't like the << operator. I
> > cannot associate the conventional meaning of the operator with 'assign
>
> all'
>
> > semantic, so I suggest that this operator is dropped completely. You can
> > provide alternative syntax for assign_all:
> >
> >      assign(v) = 1, 2, 3, 4, 5;
>
> So you mean assign_all( v ) = 1,2,3,4,5; 

Right, 'assign' was a typo.

> Seems ok. I guess if there is
> agreement about
> removing operator<<, the name hiding issue goes away for that part (but not
> for operator+=())

Exactly. For operator+= the issue is less serious because that operator is 
overloaded less often.

> > I'd suggest the declare the operators outside of class declaration. This
>
> will
>
> > give the compiler more freedom -- e.g. inlining only if maximum
>
> optimization
>
> > level is requested.
>
> I did not know there was a difference. Could you point to the place in the
> standard, please
> (in particular, I couldn't find anything in 8.3.5 and 9.5 that supports
> it).

9.3/2 says:

   A member function may be defined in its class definition, in which case it
   is an inline member function.

So defining a function inside class has the same effect as "inline" specifier.

> I think you hit a soft spot. As has been discussed in the "Forced client
> complexity" thread,
> I stumpled into that complexity. The thing is that I would like to hear
> people's apinion about
> putting these things in this library or not. If they should be in this
> library, I would like
> individual library authors to help.

I think for BGL this could be handy. Most of the time I prefer to create the 
graphs in BGL in graphviz format and parse them, but for introductionary 
examples it's too complex. Maybe we can stick to supporting only basic 
adjacency_list<> with integer vertices? In either case, it should be 
documented what happens on adding (4,6) edge in the example.

> > 3. I did not really understoon 'tuple_insert_assigner' role. Why is it
>
> needed?
>
> Basically because it does not call any constructor, but simply forwards a
> tuple of arguments
> to your callback function. (It could be explained better). This is how I
> call add_edge() in BGL
> even though add_edge() takes 3 and 4 arguments.

Ok, let me try from a different perspective: when instances of this class are 
created? At least the documentation does not say they are created anywhere...

- Volodya