[Clam-devel] TypedControls Patch
Francisco Tufró
nictuku at gmail.com
Sat May 10 09:29:21 PDT 2008
If you pass them by reference you can't do a SendControl(1) because that is
passed by copy.
So, i got confused with your mail.
Read:
> > out.SendControl(1); won't work because the signature wants a
> > reference
> > to a non-constant var! And we need it to work, for backwards
> > compatibility among other reasons.
So, i think what pau meant is that the class is passed by copy.
If you pass it by reference you can't do a SendControl(1).
So please talk with each other and tell me what to do :(
Thanks!
Francisco
On Sat, May 10, 2008 at 9:25 AM, David García Garzón <dgarcia at iua.upf.edu>
wrote:
> I fully agree with pau but Francisco's comment make me think that he
> missunderstood the point. Pau did not said passing parameters by value but
> storing copied values in the receiver. Values should be passed and returned
> to functions as const references. But the processing should store a copy. I
> guess you already do that but Pau suggested a test on it. That is, after
> sending a control, modify the sent object, the receiver should still have
> the
> old value because it was a copy.
>
>
> On Divendres 09 Maig 2008, Pau Arumí wrote:
> > On dv, 2008-05-09 at 16:36 -0300, Francisco Tufró wrote:
> > > Passing a class by copy may bring performance issues on realtime i
> > > think, that's why i did it by reference.
> > > If you want it to be by copy, i have no problem to do it by copy.
> >
> > Yes because a reason, let me explain:
> > As you say, copies can be expensive. It depends on the object size. And
> > things could be MUCH worst if a user makes a TypedControl class of
> > hierarchic objects with deep-copy semantics requiring memory allocation.
> > But using such (deep or big) objects as controls is a bad use of
> > controls, and this should be documented (at least at the doxygen class
> > doc)
> >
> > But the alternative of keeping a pointer to the original object is much
> > worst. Picture this we have a processing A sending controls to B.
> > A::Do((
> > MyType value = someVal();
> > outcontrol.SendControl(value);
> >
> > This would cause a segfault when B reads the received control, because
> > it points to non-existing var (it is out of scope).
> >
> > Restricting to send only attributes would solve this, but can not be
> > enforced. And it would also be a bad solution, it totally breaks
> > encapsulation (A and B shares the same variable).
> >
> > Some alternatives i'm thinking on now:
> > 1. Passing a copy-on-write smart-pointer. I think would be over-design
> > at this stage. Maybe later.
> > 2. Do not save the received value by default. Just provide a callback
> > method and the class decides if the value needs to be copied, to be
> > read, or to be sent out to another linked control (cascading events)
> >
> > Maybe it would be worth to do 2. But if in most cases the receiver just
> > needs to keep a copy (and wait for its Do) then it's over-design. I
> > would first implement the "copy" version.
> >
> > I'm interested to hear more opinions on 2, (David, specially)
> >
> > Pau
> >
> > > For the next patch i'll change it and add the IsConnected test and
> > > pass it.
> > > that's all for now
> > >
> > >
> > > On 5/9/08, Pau Arumí <parumi at iua.upf.edu> wrote:
> > >
> > > On dj, 2008-05-08 at 14:43 -0300, Francisco Tufró wrote:
> > > > After some hours of work i have done 2 tests and passed
> > >
> > > them.
> > >
> > > > Just check them and tell me if they're ok (and if the patch
> > >
> > > is well
> > >
> > > > done ^^)
> > > >
> > > > :)
> > >
> > > Good start! I see the code checked in by Hernan.
> > > It's is good to go in small steps. Even this simple test
> > > brings out some
> > > issues:
> > >
> > >
> > > + {
> > > + CLAM::TypedInControl<int>
> > > in("IntInControl");
> > > + CLAM::TypedOutControl<int>
> > > out("IntOutControl");
> > > + out.AddLink(in);
> > > + int number=1;
> > > + out.SendControl(number);
> > > + CPPUNIT_ASSERT_EQUAL( 1 ,
> > > in.GetLastValue() );
> > > + }
> > > +
> > >
> > > out.SendControl(1); won't work because the signature wants a
> > > reference
> > > to a non-constant var! And we need it to work, for backwards
> > > compatibility among other reasons.
> > >
> > > So change the test passing a constant, and pass the test by
> > > changing
> > > SendControl and DoControl arguments to be const references.
> > >
> > > We want controls to be passed by copy (or maybe in the future:
> > > copy-on-write smart pointers), but keeping a reference to the
> > > original
> > > object is bad.
> > >
> > > I was considering a new "sent by copy" test (e.g. send a var,
> > > change the
> > > var, check that the received value has not changed) but i
> > > realize it is
> > > not necessary, once we have changed the previous test: if you
> > > save a a
> > > const argument it necessarily implies doing copy.
> > >
> > > > When you give me the ok, i'll continue with the
> > > > "TypedInControl::IsConnected(out)" test.
> > > > Bye!
> > >
> > > Yes
> > >
> > > Pau
> > >
> > >
> > > _______________________________________________
> > > Clam-devel mailing list
> > > Clam-devel at llistes.projectes.lafarga.org
> > >
> > >
> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> > >
> > > _______________________________________________
> > > Clam-devel mailing list
> > > Clam-devel at llistes.projectes.lafarga.org
> > >
> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> >
> > _______________________________________________
> > Clam-devel mailing list
> > Clam-devel at llistes.projectes.lafarga.org
> >
> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
>
>
>
> --
> David García Garzón
> (Work) dgarcia at iua dot upf anotherdot es
> http://www.iua.upf.edu/~dgarcia <http://www.iua.upf.edu/%7Edgarcia>
>
> _______________________________________________
> Clam-devel mailing list
> Clam-devel at llistes.projectes.lafarga.org
> https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20080510/5b5cf277/attachment.html>
More information about the clam-devel
mailing list