[Clam-devel] TypedControls Patch
David García Garzón
dgarcia at iua.upf.edu
Sat May 10 05:25:09 PDT 2008
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20080510/344be1ba/attachment-0003.pgp>
More information about the clam-devel
mailing list