char* program-flow crossroads

Discussion in 'C Programming' started by Guest, May 22, 2004.

  1. Guest

    Guest Guest

    I repeatedly get into the situation where i need to take action
    accordingly to input in form of a char*, and have found two manners of
    approaching this, i'd appretiate pointers as to which is the best.
    The first approach is very small, but is kinda hardwired and gets really
    cluttery with many options. The second one takes 3 times as much code, but
    is more dynamic and easy to see what it does.


    /* Approach #1
    * Note that this one generally comes out with more code than
    * this out of some reason
    */
    void do(char* input) {
    if(strcmp(input, "foo") == 0) do_foo();
    else if(strcmp(input, "bar") == 0) do_bar();
    else if(strcmp(input, "baz") == 0) do_baz();
    else do_wtf();
    }

    /* Approach #2 */
    void do(char* input) {
    char* commands[] = {"foo", "bar", "baz"};
    enum { FOO = 0, BAR, BAZ, MAX };

    int cmd = 0;
    while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;

    switch(cmd) {
    case FOO: do_foo(); break;
    case BAR: do_bar(); break;
    case BAZ: do_baz(); break;
    default: do_wtf(); break;
    }
    }
     
    Guest, May 22, 2004
    #1
    1. Advertising

  2. Guest

    Al Bowers Guest

    wrote:
    > I repeatedly get into the situation where i need to take action
    > accordingly to input in form of a char*, and have found two manners of
    > approaching this, i'd appretiate pointers as to which is the best.
    > The first approach is very small, but is kinda hardwired and gets really
    > cluttery with many options. The second one takes 3 times as much code, but
    > is more dynamic and easy to see what it does.
    >
    >
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }


    Either approach would be acceptable.
    Looking at it from a style prespective, I tend to prefer
    approach 2. It's for the same reasons as you described.

    However, note that do is a keyword. You will need to change
    the function name. Also, since the storage to the char pointer
    argument is not modified you should use 'const char *input'.

    Example:
    #include <stdio.h>
    #include <string.h>

    void do_foo(void);
    void do_bar(void);
    void do_baz(void);
    void do_wtf(void);
    void doit(const char* input);

    int main(void)
    {
    doit("foo");
    return 0;
    }

    void do_foo(void)
    {
    puts("Fooooooooooooooo");
    return;
    }

    void do_bar(void)
    {
    puts("Barrrrrrrr");
    return;
    }

    void do_baz(void)
    {
    puts("Bazzzzzzzzzzzz");
    return;
    }

    void do_wtf(void)
    {
    puts("No command given");
    return;
    }

    void doit(const char* input)
    {
    char* commands[] = {"foo", "bar", "baz"};
    enum CMD {FIRST = 0, FOO = 0,ONE = 1, BAR = 1, BAZ,LAST } ;
    enum CMD cmd;

    for(cmd = FIRST; cmd != LAST; cmd+=ONE)
    if(strcmp(commands[cmd], input)==0) break;

    switch(cmd)
    {
    case FOO: do_foo(); break;
    case BAR: do_bar(); break;
    case BAZ: do_baz(); break;
    default: do_wtf(); break;
    }
    }


    --
    Al Bowers
    Tampa, Fl USA
    mailto: (remove the x to send email)
    http://www.geocities.com/abowers822/
     
    Al Bowers, May 22, 2004
    #2
    1. Advertising

  3. Guest

    Stephen L. Guest

    wrote:
    >


    [ snip ]

    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }


    This approach seems the easiest to maintain
    of the two. A couple of suggestions -

    If the number of commands isn't too many,
    I put them in the list sorted, and use
    a less than or equal to for the comparison.

    OR

    If some of the commands are more heavily used
    over the other commands, then put them
    in the beginning of the `commands' array.

    I'd also use `(sizeof(commands) / sizeof(commands[0]))'
    instead of `MAX', and make the enum a "type" -

    enum { FOO = 0, BAR, BAZ, } cmd;

    This way (e.g. gcc) will warn if a new item is added
    to the `commands' and the enum type, but is not
    added to the switch statement. Kinda a minor point, though.

    ---

    If you're _really_ feeling crazy, you could marry
    the command string and it's function pointer handler
    into a structure, and build an array of those.

    Now you can eliminate the enum (since it's only
    acting as a function pointer index anyway, and
    a possibly long switch statement. This, of
    course, assumes all of the handlers can have
    the same or compatible prototype (which your example does).


    HTH,

    Stephen
     
    Stephen L., May 22, 2004
    #3
  4. <> writes:

    > I repeatedly get into the situation where i need to take action
    > accordingly to input in form of a char*, and have found two manners of
    > approaching this, i'd appretiate pointers as to which is the best.
    > The first approach is very small, but is kinda hardwired and gets really
    > cluttery with many options. The second one takes 3 times as much code, but
    > is more dynamic and easy to see what it does.
    >
    >
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };


    (You might want to rename `MAX' to something like `NUM', as it
    represents the number of possible commands, not the maximum command
    number.)

    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }


    Both approaches have the disadvantage that they possibly compare the
    input string with /every/ command string. That can get quite slow
    unless you really only have very few (three, say) commands.

    If you don't have too many commands, you could use a `switch' statement
    on the first character, which can already significantly reduce the
    number of necessary string comparisons:

    void do (const char *const input)
    {
    switch (*input)
    {
    case 'b':
    if (strcmp (input + 1, "ar") == 0)
    do_bar ();
    else if (strcmp (input + 1, "az") == 0)
    do_baz ();
    else
    do_wtf ();
    break;

    case 'f':
    if (strcmp (input + 1, "oo") == 0)
    do_foo ();
    else
    do_wtf ();
    break;

    default:
    do_wtf ();
    break;
    }
    }

    Otherwise, your best choice is probably a hash function. If the command
    strings are fixed (i.e. their number and content both don't change at
    runtime), you could use a "perfect hash function", which never needs
    more than at most /one/ string comparison. (Google for the term if you
    need more information.)

    Martin


    --
    ,--. Martin Dickopp, Dresden, Germany ,= ,-_-. =.
    / ,- ) http://www.zero-based.org/ ((_/)o o(\_))
    \ `-' `-'(. .)`-'
    `-. Debian, a variant of the GNU operating system. \_/
     
    Martin Dickopp, May 22, 2004
    #4
  5. Guest

    Malcolm Guest

    <> wrote
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >

    This is straightforward, and relatively easy to maintain.
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }
    >

    This is a bit more difficult to maintain, because you've got to work through
    the logic of the function to see how to modify it.
    However it does have the advantage that all the strings literal are in one
    place, which might help in internationalisation. The other advantage is
    that, should the number of commands be large and the function time-critical,
    it is easy to replace your linear search with an alphabetical binary serach.
     
    Malcolm, May 22, 2004
    #5
  6. Guest

    rgb Guest

    <> wrote in message news:<7sJrc.33$>...
    > I repeatedly get into the situation where i need to take action
    > accordingly to input in form of a char*, and have found two manners of
    > approaching this, i'd appretiate pointers as to which is the best.
    > The first approach is very small, but is kinda hardwired and gets really
    > cluttery with many options. The second one takes 3 times as much code, but
    > is more dynamic and easy to see what it does.
    >
    >
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }


    I'm sure somebody will find this controverial - to me it's a no-brainer.
    #2 is like waxing a trash truck.
    1) it's less clear 2) harder to maintain 3) takes more cpu.
    You might think is't more clear but it only 'looks' more clear.
    What keeps your enum in sync with your char* array?
    Were there some performance advantage to the 2nd you would need
    to weigh maintainability vs speed but there isn't since #2 forces
    unnecessary duplication of decisions into the algorithm as well.
    No brainer.
    The only advantage of #2 is the code can be made to 'look pretty'.
    This forces the next guy to maintain both code and 'prettyness'
    (more work for maintainer) and then you waste cpu cycles too.

    Unless you paper your walls with the listings #2 is no good in my book.
     
    rgb, May 22, 2004
    #6
  7. Guest

    Peter Ammon Guest

    wrote:

    > I repeatedly get into the situation where i need to take action
    > accordingly to input in form of a char*, and have found two manners of
    > approaching this, i'd appretiate pointers as to which is the best.
    > The first approach is very small, but is kinda hardwired and gets really
    > cluttery with many options. The second one takes 3 times as much code, but
    > is more dynamic and easy to see what it does.
    >
    >
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }


    #2 is pointless and difficult to maintain elaboration. To reduce code
    size and improve clarity in #1, consider a macro.

    #define INPUT_IS(a) ( ! strcmp(input, (#a)))

    void do_command(char* input) {
    if (INPUT_IS(foo)) do_foo();
    else if (INPUT_IS(bar)) do_foo();
    else if (INPUT_IS(baz)) do_baz();
    else do_wtf();
    }

    --
    Pull out a splinter to reply.
     
    Peter Ammon, May 23, 2004
    #7
  8. Peter Ammon wrote:

    > wrote:
    >
    >> I repeatedly get into the situation where i need to take action
    >> accordingly to input in form of a char*, and have found two manners of
    >> approaching this, i'd appretiate pointers as to which is the best.
    >> The first approach is very small, but is kinda hardwired and gets really
    >> cluttery with many options. The second one takes 3 times as much code,
    >> but
    >> is more dynamic and easy to see what it does.
    >>
    >>
    >> /* Approach #1
    >> * Note that this one generally comes out with more code than
    >> * this out of some reason */
    >> void do(char* input) {
    >> if(strcmp(input, "foo") == 0) do_foo();
    >> else if(strcmp(input, "bar") == 0) do_bar();
    >> else if(strcmp(input, "baz") == 0) do_baz();
    >> else do_wtf();
    >> }
    >>
    >> /* Approach #2 */
    >> void do(char* input) {
    >> char* commands[] = {"foo", "bar", "baz"};
    >> enum { FOO = 0, BAR, BAZ, MAX };
    >>
    >> int cmd = 0;
    >> while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >>
    >> switch(cmd) {
    >> case FOO: do_foo(); break;
    >> case BAR: do_bar(); break;
    >> case BAZ: do_baz(); break;
    >> default: do_wtf(); break;
    >> }
    >> }

    >
    >
    > #2 is pointless and difficult to maintain elaboration. To reduce code
    > size and improve clarity in #1, consider a macro.
    >
    > #define INPUT_IS(a) ( ! strcmp(input, (#a)))
    >
    > void do_command(char* input) {
    > if (INPUT_IS(foo)) do_foo();
    > else if (INPUT_IS(bar)) do_foo();
    > else if (INPUT_IS(baz)) do_baz();
    > else do_wtf();
    > }
    >


    Note that either of the original sets of code could
    easily be generated by some kind or pre-processor
    given the input file
    foo
    bar
    baz

    (e.g. "awk" in the Unix world)

    Then you would only have to maintain that file
    (assuming that if you added "grok" to the list
    you added a routine do_grok() which the linker
    should catch if you did not).

    6 of one and half a dozen of another as to
    which of the above code samples it generated
    AFAICT.

    --
    "It is impossible to make anything foolproof
    because fools are so ingenious"
    - A. Bloch
     
    Nick Landsberg, May 23, 2004
    #8
  9. <> wrote in message news:<7sJrc.33$>...
    > I repeatedly get into the situation where i need to take action
    > accordingly to input in form of a char*, and have found two manners of
    > approaching this, i'd appretiate pointers as to which is the best.
    > The first approach is very small, but is kinda hardwired and gets really
    > cluttery with many options. The second one takes 3 times as much code, but
    > is more dynamic and easy to see what it does.
    >
    >
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }


    Approach #3: if all the functions have the same signature, then modify approach 2...

    % type functbl.c
    #include <stdio.h>
    #include <string.h>

    void do_foo() { puts("foo()"); }
    void do_bar() { puts("bar()"); }
    void do_baz() { puts("baz()"); }
    void do_wtf() { puts("wtf()"); }

    struct func_tbl_t
    {
    const char *name;
    void (*func)(void);
    };

    struct func_tbl_t func_tbl[] =
    {
    { "foo", do_foo },
    { "bar", do_bar },
    { "baz", do_baz }
    };

    void do_func(const char *f)
    {
    size_t i;
    for (i = 0; i < sizeof func_tbl / sizeof *func_tbl; i++)
    if (strcmp(func_tbl.name, f) == 0)
    {
    func_tbl.func();
    return;
    }
    do_wtf();
    }

    int main(int argc, char **argv)
    {
    if (argc)
    while (argv++, --argc)
    do_func(*argv);

    return 0;
    }

    % acc functbl.c -o functbl.exe

    % functbl foo hello bar baz biz
    foo()
    wtf()
    bar()
    baz()
    wtf()

    %

    --
    Peter
     
    Peter Nilsson, May 23, 2004
    #9
  10. wrote:
    >
    > I repeatedly get into the situation where i need to take action
    > accordingly to input in form of a char*, and have found two manners of
    > approaching this, i'd appretiate pointers as to which is the best.
    > The first approach is very small, but is kinda hardwired and gets really
    > cluttery with many options. The second one takes 3 times as much code, but
    > is more dynamic and easy to see what it does.
    >
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }
    >

    In your second option, you can put all your commands in a sting:

    enum {FOO = 1, BAR, BAZ, MAX};
    char *commands = "nul&foo&bar&baz&";

    and replace the "while" loop with a call to "strstr()" and some
    arithmetic:

    cmd = strstr(commands,input);

    if(cmd)
    cmd = (cmd - commands) / 4;

    "cmd" will contain the number of the proper command, or zero if
    the command given is *not* in the list of commands. Although in
    your example it would *not* be a problem, the "&" delimiter in
    the "commands" string keeps two commands from running together.

    --
    +----------------------------------------------------------------+
    | Charles and Francis Richmond richmond at plano dot net |
    +----------------------------------------------------------------+
     
    Charles Richmond, May 23, 2004
    #10
  11. Guest

    James Kanze Guest

    <> writes:

    |> I repeatedly get into the situation where i need to take action
    |> accordingly to input in form of a char*, and have found two manners
    |> of approaching this, i'd appretiate pointers as to which is the
    |> best. The first approach is very small, but is kinda hardwired and
    |> gets really cluttery with many options. The second one takes 3 times
    |> as much code, but is more dynamic and easy to see what it does.

    |> /* Approach #1
    |> * Note that this one generally comes out with more code than
    |> * this out of some reason
    |> */
    |> void do(char* input) {
    |> if(strcmp(input, "foo") == 0) do_foo();
    |> else if(strcmp(input, "bar") == 0) do_bar();
    |> else if(strcmp(input, "baz") == 0) do_baz();
    |> else do_wtf();
    |> }

    |> /* Approach #2 */
    |> void do(char* input) {
    |> char* commands[] = {"foo", "bar", "baz"};
    |> enum { FOO = 0, BAR, BAZ, MAX };

    |> int cmd = 0;
    |> while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;

    |> switch(cmd) {
    |> case FOO: do_foo(); break;
    |> case BAR: do_bar(); break;
    |> case BAZ: do_baz(); break;
    |> default: do_wtf(); break;
    |> }
    |> }

    The second one requires duplicate information, which can get out of
    synch. It should be avoided.

    --
    James Kanze
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France +33 (0)1 30 23 00 34
     
    James Kanze, May 23, 2004
    #11
  12. Guest

    red floyd Guest

    <> wrote in message news:<7sJrc.33$>...
    > [redacted
    >
    > /* Approach #1
    > * Note that this one generally comes out with more code than
    > * this out of some reason
    > */
    > void do(char* input) {
    > if(strcmp(input, "foo") == 0) do_foo();
    > else if(strcmp(input, "bar") == 0) do_bar();
    > else if(strcmp(input, "baz") == 0) do_baz();
    > else do_wtf();
    > }
    >
    > /* Approach #2 */
    > void do(char* input) {
    > char* commands[] = {"foo", "bar", "baz"};
    > enum { FOO = 0, BAR, BAZ, MAX };
    >
    > int cmd = 0;
    > while(cmd < MAX) if(strcmp(commands[cmd++], input)==0) break;
    >
    > switch(cmd) {
    > case FOO: do_foo(); break;
    > case BAR: do_bar(); break;
    > case BAZ: do_baz(); break;
    > default: do_wtf(); break;
    > }
    > }


    Your code (either approach) is uncompilable as-is. "do" is a keyword in C.
     
    red floyd, May 23, 2004
    #12
    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. wwj
    Replies:
    7
    Views:
    558
  2. wwj
    Replies:
    24
    Views:
    2,521
    Mike Wahler
    Nov 7, 2003
  3. lovecreatesbeauty
    Replies:
    1
    Views:
    1,064
    Ian Collins
    May 9, 2006
  4. utab
    Replies:
    18
    Views:
    1,664
    Diego Martins
    Apr 27, 2006
  5. Jack Dowson
    Replies:
    0
    Views:
    458
    Jack Dowson
    May 7, 2007
Loading...

Share This Page