«

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;
}
}

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/

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

<> 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. \_/

<> 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.

<> 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.

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 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

<> 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[i].name, f) == 0)
{
func_tbl[i].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

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 |
+----------------------------------------------------------------+