[Clam-devel] TypedControls: asking for advice and opinions

[Hernán Ordiales]

Hi all,

On Fri, Jul 11, 2008 at 3:12 PM, Francisco Tufró <nictuku at gmail.com> wrote:
> Hi, I've been trying to do the migration to TypedControls and we've some
> problems.
> Since the *ControlRegistry now is defined as std::list<BaseTyped*Control*>
> the functions Get() and GetByNumber now return a BaseTyped*Control instead
> of a *Control.
> The same is happening with Get*Controls() which now returns a list<Base..>
> instead of a list<*Control *>
> So, every single code line using Get*Controls as well as Get() and
> GetByNumber() has to be replaced.
> As Pau proposed i wrote the function that connected a In to a temporary out
> and sends the value, but that was just one of the appearances of the
> problem.
[snip]
>  - For the already known problem (the one of this thread) i wrote the
> following function
> (i made also a version where an int is passed instead of the name, but is
> essentially the same, for those places where GetByNumber was used):
>     void SendFloatToInControl(Processing & receiver, const std::string &
> inControlName, float value){
>         TypedOutControl<float> controlSender;
>         controlSender.AddLink(receiver.GetInControls().Get(inControlName));
>         controlSender.SendControl(value);
>     }

> I tried to solve every one of these problems with the better solution i
> thought, now i present these solutions:

I'll reorder the problems you exposed and proposed solutions:

> Those functiones were used in other circumstances:
>   - Assignations: MIDIIO/MIDIKeyboard.cxx:        OutControl& outVelocity =
> mNoteIn.GetOutControls().GetByNumber(2);

> The first i had to do a static cast, i don't like this solution, but the
> code needs to be rewritten to avoid it, this happends also in NetworkEditor
> and Annotator.

This seems the more ugly...

>   - To access in controls defined in parent classes:
> SpectralSpread.hxx:
> GetInControl("Amount").DoControl(100);

> The second was solved with this function also, but passed *this as argument.

ok, but if controls are protected (or public) i think you can use the
var name instead (like third case) and if not, i think they should...

>   - also (don't know why) in the Initialization function of some processings
> (where the controls were accesible by its own variable name, but anyways
> they were referenced with Get*Controls:
> FrameTransformation.hxx:
> GetInControl("Amount").DoControl(0.);

> The third was solved replacing the lines using the variable name.

ok for me.

> Another of these problems was that GetInControl was used to ask for
> GetLastValue of an InControl, for this i wrote:
> float GetFloatFromInControl(Processing & proc, const std::string &
> inControlName){
>         InControl& in = (InControl&)(proc.GetInControl(inControlName));
>         return in.GetLastValue();
>     }
> This needed a typecast, but static. I'm not so sure about this solution.
> This was needed in the SMSDeesser lines 44-47.

ok

> Also there were some places that used GetOutControl to do a SendControl,
> with these i used this function:
>
>     void SendFloatToOutControl(Processing & sender, int inControlIndex,
> float value){
>         OutControl& out =
> (OutControl&)(sender.GetOutControls().GetByNumber(inControlIndex));
>         out.SendControl(value);
>     }
> Again, a cast was needed, used in MIDIInControl.hxx.

ok

> Hope you can help me with this and tell me how to best solve the migration
> problem.
> bye!

Well, actually we already discussed those problems in private, now I'm
interested to hear Pau and David opinions about all these cases since
are very close to the clam core and I'd like to know if have better
ideas or think the patch is ready for commitment...
Franciso's patch with these solutions compiles[1] and pass all tests
and he was already playing a bit with networks at a (patched) NE and
clam lib.

[1] at least NE and clam lib, I think is pending a similar review to
annotator and smstools but i hope that no new use case emerge

cheers
-- 
Hernán
http://h.ordia.com.ar
GnuPG: 0xEE8A3FE9