Library design opinions...

Discussion in 'C++' started by rep_movsd, Jun 27, 2008.

  1. rep_movsd

    rep_movsd Guest

    Hi

    I have developed a simple free library that is used for creating and
    managing Directshow filter graphs, that I (and possibly several
    others ) have been using for more than a year now...

    Recently I thought of rewriting it with a more "intuitive" usage style
    ( mostly to satisfy my desire to play with template metaprogramming ),
    and gain a bit of static error checking ( and making the user code
    look like magic )

    Heres some equivalent code for the old and new libraries ( its a bit
    specific to the dshow framework, but it is fairly obvious what the
    code does, also all the includes etc have been omitted )

    Old library usage :
    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////

    void BuildGraph(CGraph &g)
    {
    g.Connect(L"SOURCE", L"TEE");
    g.Connect(L"TEE", L"RENDERER");
    g.Connect(L"TEE", L"RENDERER2", 1, 0); // connect 2nd output pin
    of tee to renderer2's first pin
    }

    void BuildGraphBad(CGraph &g)
    {
    g.Connect(L"SOURCE", L"TEE");
    g.Connect(L"TEE", L"RENDERERER"); // Runtime error "filter
    not found "
    g.Connect(L"TEE", L"RENDERER2", 1, 0);
    }

    int main(int argc, char **argv)
    {
    CGraph g;
    g.AddSourceFilter(argv[1], L"SOURCE"); // Create a source
    filter based on filename
    g.AddFilter(CLSID_InfTee, L"TEE"); // Create a Tee
    g.AddFilter(CLSID_VideoRenderer, L"RENDERER"); // Create
    renderers
    g.AddFilter(CLSID_VideoRenderer, L"RENDERER2");
    BuildGraph(g);
    BuildGraphBad(g);
    };

    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////


    New library usage:
    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////

    DECLARE_FILTER(SOURCE);
    DECLARE_FILTER(TEE);
    DECLARE_FILTER(RENDERER);
    DECLARE_FILTER(RENDERER2);

    void BuildGraph()
    {
    FILTER(SOURCE);
    FILTER(TEE);
    FILTER(RENDERER);
    FILTER(RENDERER2);

    SOURCE >> TEE >> RENDERER; // Chain the filters
    TEE >> RENDERER2; // Add an extra renderer
    }

    void BuildGraphBad()
    {
    FILTER(SOURCE);
    FILTER(TEE);
    FILTER(RENDERERER); // Misspelling - compile error

    SOURCE >> TEE >> RENDERER; // Compile error
    }

    int main(int argc, char **argv)
    {
    CGraf g;
    FILTER(SOURCE);
    FILTER(TEE);
    FILTER(RENDERER);
    FILTER(RENDERER2);

    SOURCE = argv[1]; // Create a source filter
    based on filename
    TEE = CLSID_InfTee; // Create filters based on
    CLSIDs
    RENDERER = CLSID_VideoRenderer;
    RENDERER2 = CLSID_VideoRenderer;

    g += SOURCE, TEE, RENDERER, RENDERER2; // FIlters get added to the
    graph
    BuildGraph();
    };

    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////


    Some of the advantages of the new style are :
    1) FIlter names are identifiers, so u get intellisense and compile
    time checking
    2) A filter with a given name always refers to the same object, so
    there is no need to store a bunch of pointers to filter objects and
    pass them around various functions ( or worse yet keep them global as
    most directshow code does )
    3) The operator overloading makes the code very readable,

    The only disadvantage I see is that the implementation is complex and
    I am not 100% confident that my code is perfect.

    I was wondering to overload the unary minus operator to remove
    filters from the graph and the divide by operator to disconnect
    filters. I would also add some manipulator stuff ( like in iostreams)
    to control the way filters are
    connected with >> e.g.

    // Remove renderer2 from graph
    g -= RENDERER2;

    // Connect the first audio pin of the source to the renderer
    SOURCE >> pinType(MEDIATYPE_Audio) >> AUDIORENDER;

    // Disconnect any connection betwen these 2 filters
    SOURCE / AUDIORENDER;

    Any opinions, suggestions?

    Vivek
    rep_movsd, Jun 27, 2008
    #1
    1. Advertising

  2. rep_movsd

    rep_movsd Guest

    > Notes:
    > [1] That means, the all uppercase. Are those really macros? If so, get rid of them.


    FILTER and DECLARE_FILTER are macros...
    The rest are identifiers....
    rep_movsd, Jun 27, 2008
    #2
    1. Advertising

  3. rep_movsd

    Boris Guest

    On Fri, 27 Jun 2008 09:09:18 +0200, rep_movsd <> wrote:

    > [...]Recently I thought of rewriting it with a more "intuitive" usage
    > style
    > ( mostly to satisfy my desire to play with template metaprogramming ),
    > and gain a bit of static error checking ( and making the user code
    > look like magic )


    Your new code doesn't look like magic to me - it looks more like it has
    been deliberately obfuscated. Once you start using the library in another
    use case where you don't do exactly what is explained in the documentation
    you have to fiddle around with all the macros trying to understand what's
    going on there in detail. It's more useful to know what is a class, a
    function, an object etc. as every C++ programmer understands then
    immediately what they can do.

    Boris

    > [...]
    Boris, Jun 27, 2008
    #3
  4. rep_movsd

    rep_movsd Guest

    On Jun 27, 1:06 pm, Boris <> wrote:

    > Your new code doesn't look like magic to me - it looks more like it has  
    > been deliberately obfuscated. Once you start using the library in another  
    > use case where you don't do exactly what is explained in the documentation  
    > you have to fiddle around with all the macros trying to understand what's  
    > going on there in detail. It's more useful to know what is a class, a  
    > function, an object etc. as every C++ programmer understands then  
    > immediately what they can do.
    >
    > Boris


    Yes, I agree that the declaration macros have quite complex
    implementations and are opaque, But what about the use of overloaded
    operators to make the connections etc?
    Does that look bad too?

    Vivek
    rep_movsd, Jun 27, 2008
    #4
  5. rep_movsd

    Boris Guest

    On Fri, 27 Jun 2008 13:08:01 +0200, rep_movsd <> wrote:

    > [...]Yes, I agree that the declaration macros have quite complex
    > implementations and are opaque, But what about the use of overloaded
    > operators to make the connections etc?
    > Does that look bad too?


    I try either to reuse an idea from the standard (for example everybody has
    an idea what operator<< and operator>> do because of streams) or make sure
    that the usage of operators feels natural (which might be more difficult
    as what is natural for me might not be natural for someone else). Your
    library seems to use operator>> to chain filters and operator+= to add
    filters? I probably wouldn't use overloaded operators here: operator>>
    makes me think that there is a data flow while in fact it's just a
    chaining of filters (if it was a lambda function it would make more sense
    maybe :). And when operator+= is overloaded you wonder if and how
    operator+ fits into the schema.

    Boris
    Boris, Jun 27, 2008
    #5
  6. rep_movsd

    Looney Guest

    On Jun 27, 5:09 pm, rep_movsd <> wrote:
    > Hi
    >
    > I have developed a simple free library that is used for creating and
    > managing Directshow filter graphs, that I (and possibly several
    > others ) have been using for more than a year now...
    >
    > Recently I thought of rewriting it with a more "intuitive" usage style
    > ( mostly to satisfy my desire to play with template metaprogramming ),
    > and gain a bit of static error checking ( and making the user code
    > look like magic )
    >
    > Heres some equivalent code for the old and new libraries ( its a bit
    > specific to the dshow framework, but it is fairly obvious what the
    > code does, also all the includes etc have been omitted )
    >
    > Old library usage :
    > //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
    >
    > void BuildGraph(CGraph &g)
    > {
    > g.Connect(L"SOURCE", L"TEE");
    > g.Connect(L"TEE", L"RENDERER");
    > g.Connect(L"TEE", L"RENDERER2", 1, 0); // connect 2nd output pin
    > of tee to renderer2's first pin
    >
    > }
    >
    > void BuildGraphBad(CGraph &g)
    > {
    > g.Connect(L"SOURCE", L"TEE");
    > g.Connect(L"TEE", L"RENDERERER"); // Runtime error "filter
    > not found "
    > g.Connect(L"TEE", L"RENDERER2", 1, 0);
    >
    > }
    >
    > int main(int argc, char **argv)
    > {
    > CGraph g;
    > g.AddSourceFilter(argv[1], L"SOURCE"); // Create a source
    > filter based on filename
    > g.AddFilter(CLSID_InfTee, L"TEE"); // Create a Tee
    > g.AddFilter(CLSID_VideoRenderer, L"RENDERER"); // Create
    > renderers
    > g.AddFilter(CLSID_VideoRenderer, L"RENDERER2");
    > BuildGraph(g);
    > BuildGraphBad(g);
    >
    > };
    >
    > //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
    >
    > New library usage:
    > //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
    >
    > DECLARE_FILTER(SOURCE);
    > DECLARE_FILTER(TEE);
    > DECLARE_FILTER(RENDERER);
    > DECLARE_FILTER(RENDERER2);
    >
    > void BuildGraph()
    > {
    > FILTER(SOURCE);
    > FILTER(TEE);
    > FILTER(RENDERER);
    > FILTER(RENDERER2);
    >
    > SOURCE >> TEE >> RENDERER; // Chain the filters
    > TEE >> RENDERER2; // Add an extra renderer
    >
    > }
    >
    > void BuildGraphBad()
    > {
    > FILTER(SOURCE);
    > FILTER(TEE);
    > FILTER(RENDERERER); // Misspelling - compile error
    >
    > SOURCE >> TEE >> RENDERER; // Compile error
    >
    > }
    >
    > int main(int argc, char **argv)
    > {
    > CGraf g;
    > FILTER(SOURCE);
    > FILTER(TEE);
    > FILTER(RENDERER);
    > FILTER(RENDERER2);
    >
    > SOURCE = argv[1]; // Create a source filter
    > based on filename
    > TEE = CLSID_InfTee; // Create filters based on
    > CLSIDs
    > RENDERER = CLSID_VideoRenderer;
    > RENDERER2 = CLSID_VideoRenderer;
    >
    > g += SOURCE, TEE, RENDERER, RENDERER2; // FIlters get added to the
    > graph
    > BuildGraph();
    >
    > };
    >
    > //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
    >
    > Some of the advantages of the new style are :
    > 1) FIlter names are identifiers, so u get intellisense and compile
    > time checking
    > 2) A filter with a given name always refers to the same object, so
    > there is no need to store a bunch of pointers to filter objects and
    > pass them around various functions ( or worse yet keep them global as
    > most directshow code does )
    > 3) The operator overloading makes the code very readable,
    >
    > The only disadvantage I see is that the implementation is complex and
    > I am not 100% confident that my code is perfect.
    >
    > I was wondering to overload the unary minus operator to remove
    > filters from the graph and the divide by operator to disconnect
    > filters. I would also add some manipulator stuff ( like in iostreams)
    > to control the way filters are
    > connected with >> e.g.
    >
    > // Remove renderer2 from graph
    > g -= RENDERER2;
    >
    > // Connect the first audio pin of the source to the renderer
    > SOURCE >> pinType(MEDIATYPE_Audio) >> AUDIORENDER;
    >
    > // Disconnect any connection betwen these 2 filters
    > SOURCE / AUDIORENDER;
    >
    > Any opinions, suggestions?
    >
    > Vivek


    Hi Vivek,
    I would advice that only overload operators which your library's users
    would find intuitive, also while following the normal semantics of
    those operators. Remember the goal of your rewrite is to make a more
    user friendly library, your library's users all ready know that you
    can write nifty code, so do not just overload operators because u can.

    Also I am a big fan of c++ Meta programming, though i do not
    necessarily see that you can well use a lot of it for the task at
    hand . You might be able to create cool types which know at compile
    time what filters to add, but u still need to make runtime calls to
    Direct show library to actually setup the filter graph. Meta
    programming usually is more usefull when all or majority of the info
    is available at compile time and you can get compiler to evaluate
    expressions, inline calls and un-roll loops etc to generate faster
    code. In your case say even if you had a types to represent a filter
    at compile time, you still actually do need to setup the filter graph
    at runtime with DS library, also consider what happens if the user of
    your library wants to iterate over all the available filter types at
    runtime and decide which ones to actually add to the filter graph. A
    Meta programming solution is set out in stone and would fall short to
    meet the user's needs.

    Please do not be offended as i do not intend to offend, but i too like
    others prefer the first code snippet over the second as upon a quick
    glance, it's much clearer to me what the code is doing. The second
    code snippet just seems to create unnecessary confusion.
    My 2 cents
    Looney, Jun 27, 2008
    #6
  7. rep_movsd

    rep_movsd Guest

    >
    > I try either to reuse an idea from the standard (for example everybody has  
    > an idea what operator<< and operator>> do because of streams) or make sure  
    > that the usage of operators feels natural (which might be more difficult  
    > as what is natural for me might not be natural for someone else). Your  
    > library seems to use operator>> to chain filters and operator+= to add  
    > filters? I probably wouldn't use overloaded operators here: operator>>  
    > makes me think that there is a data flow while in fact it's just a  
    > chaining of filters (if it was a lambda function it would make more sense  
    > maybe :). And when operator+= is overloaded you wonder if and how  
    > operator+ fits into the schema.
    >
    > Boris


    Well in dshow architecture, a graph is a bag of filters and the
    connections will have data flow through them ( in the direction of the
    '>>' ). Filters are more like digital ICs with input and output pins
    ( the term filter isnt so fitting in fact )

    Graphs are always directed, so i thought using >> to hook up filters
    together would be intuitive.

    The fact that you saw it as a "data flow" is actually a good thing :D

    Yes, now that you mention it, the meaning of + is vague if += adds
    filters to the graph.

    Vivek
    rep_movsd, Jun 27, 2008
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. John Doe
    Replies:
    3
    Views:
    390
    Sandeep
    Jul 9, 2003
  2. Tim H

    design opinions requested

    Tim H, Jun 3, 2007, in forum: C++
    Replies:
    15
    Views:
    539
    Daniel T.
    Jun 4, 2007
  3. max w.
    Replies:
    4
    Views:
    645
  4. PaulMac

    Web Service App design opinions

    PaulMac, Oct 1, 2003, in forum: ASP .Net Web Services
    Replies:
    4
    Views:
    146
    Yan-Hong Huang[MSFT]
    Oct 3, 2003
  5. Kirk Haines
    Replies:
    6
    Views:
    84
    Sean O'Dell
    Jun 30, 2004
Loading...

Share This Page