Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#6 MarSystem/Control path confusion

open
None
5
2012-09-17
2010-06-10
Stefaan Lippens
No

Current MarSystem/Control paths are something like:
"Accumulator/accumulator/Series/frameFeatureExtractor/Series/soundFileHopper/mrs_natural/hopSize"

IMHO this style is confusing because it looks like a path down a hierarchy, as we are used to with the file systems on our computers , but with a strange twist: each steps down the hierarchy corresponds with two components in the "path": a type and a name: "type/name/type/name/type/name".

First proposal/question/request:
Wouldn't it improve readability to change the separator between the type and name with a colon ("type:name")? For example:
"Accumulator:accumulator/Series:frameFeatureExtractor/Series:soundFileHopper/mrs_natural:hopSize"
This colon-form could be supported together with the traditional all-slash-form, which could become deprecated but still allowed to make sure old code stull works.

Second proposal/question/request:
I never understood the need to specify the type in a MarSystem/Control path, as it is redundant. Is it for clarity? Is it to be able to do a consistency check?
Anyway, why not just use the following?
"accumulator/frameFeatureExtractor/soundFileHopper/hopSize"

Of course, this would breaks compatibility, unless a different separator would be introduced, like ">":
"accumulator>frameFeatureExtractor>soundFileHopper>hopSize"

any thoughts?

Discussion

  • The control's path is inspired in the way Open Sound Control (OSC) implements its messages, and also in UNIX FS, for sure. So, for someone familiar with OSC and UNIX (and there is an increasing number of people woking in something sound related that are becoming familiar with those two "platforms"), this control path naming should not be so akward (although, it is verbose).

    Keeping the datatype specifier will make life easier if we want to convert from a MarControl to, for e.g., an OSC message.

    I'm assiging this one to George, so he can have a say (and reassign to anyone he decides).

     
  • Funny, I had the exact same idea:) I kept a list of feature requests and this is what I wrote down. In addition I think the often used type casts are also redundant. This is what I had written down:

    Remove types in strings and remove casting for getters and setters.
    In my opinion the way getters and setters of variables / controls are done in Marsyas is too elaborate and hard to read. I suggest we change the strings so that the types are not mentioned. This would require for each level to have types with unique names, but I think that is no issue since that's what you'd want anyway.

    Syntax like this:

    mrs_real srate = recordNet->getctrl("AudioSource/asrc/mrs_real/israte") ->to<mrs_real>();

    could be:

    mrs_real srate = recordNet->getValue("asrc/israte");

    Of course to make it even more readable you'd have to choose sensible names for marsystems and controls:
    mrs_real sampleRate = recordNet->getValue("audioSource/sampleRate");

    This would make a HUGE difference to the readability of the Marsyas code in general.

    About the OSC thing. Converting a marcontrol to an osc message could be just as easy, since you still know the type of each level you'll just have to make a function to translate it.

    Also we don't HAVE to get rid of the current string format at all. Just make it an option.

     
  • <quote>
    mrs_real srate = recordNet->getctrl("AudioSource/asrc/mrs_real/israte")
    ->to<mrs_real>();

    could be:

    mrs_real srate = recordNet->getValue("asrc/israte");
    </quote>

    I just realized that his syntax isn't possible because c++ doesn't support function overloading on return type. A solution would be to pass the variable that you want as a return value to the function by reference:

    mrs_real samplerate;
    recordNet->getValue(samplerate, "asrc/israte");

    If the type of arc/israte is not an mrs_real, I think an exception should be thrown, or you could try an implicit cast first.

     
  • Hi Thijs, about:
    mrs_real srate = recordNet->getctrl("AudioSource/asrc/mrs_real/israte")->to<mrs_real>();
    and the alternative you propose:
    mrs_real samplerate;
    recordNet->getValue(samplerate, "asrc/israte");

    I'm not a fan of mixing input and output parameters in the function arguments. I think it's very bad for code readability and would still prefer the template trick "->to<T>()".

    However, you noted that C++ does not support function overloading on return type. Instead of depending on overloading, we could make custom getters per return type. e.g.

    mrs_real srate = recordNet->getRealValue("asrc/israte");
    

    which could be just a thin/inline wrapper around getctrl()->to<mrs_real>()

     
  • I don't mind putting return types in functions, as long as its consistently the first parameter. You signature could be clear if you use const in combination with proper variable names. But I understand if people prefer a different way.

    I think your suggestion is ok, although I would name the functions getValueReal instead getRealValue, since that would automatically group the functions by name.

    Personally I would prefer to use a template function then, since it would minimize the amound of functions and gives you the possibility to cast to a different type in one go, should you need to for other interfaces. So something like:

    mrs_real samplerate = recordNet->getValue<mrs_real>("asrc/israte");

    then if you need an int for a different API you could write:
    int samplerate = recordNet->getValue<int>("asrc/israte");

    The compiler can detect at compile time if you're trying to make a valid cast.

     


Anonymous


Cancel   Add attachments