# Code review of using case and switch macros for string

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define SWITCH(S) char *_S = S; if (0)
#define CASE(S) } else if (strcmp(_S, S) == 0) {switch(1) { case 1
#define BREAK }
#define DEFAULT } else {switch(1) { case 1

int main()
{
char buf[256];

printf("\nString - Enter your string: ");
scanf ("%s", buf);

SWITCH (buf) {
CASE ("abcdef"):
printf ("B1!\n");
BREAK;
CASE ("ghijkl"):
printf ("C1!\n");
BREAK;
DEFAULT:
printf ("D1!\n");
BREAK;
}
}

-
If you want a different syntax, use a different language. Don't try to bend C to your taste by misusing the preprocessor. It will just cause problems. –  William Morris Jan 9 '13 at 15:56

switch is usually assumed to have nearly O(1). I'm suggesting to introduce additional level for parsing. It is known as lexical analysis. You parse you fixed set of cases to enum values and then switch as you want.

enum { B1, C1, D1 } token_t;

token_t lexer(const char *s)
{
// TODO: consider hash table here
static struct entry_s {
const char *key;
token_t token;
} token_table[] = {
{ "abcdef", B1 },
{ "ghijkl", C1 },
{ NULL, D1 },
};
struct entry_s *p = token_table;
for(; p->key != NULL && strcmp(p->key, s) != 0; ++p);
return p->token;
}

int main()
{
char buf[256];

printf("\nString - Enter your string: ");
scanf ("%s", buf);

switch(lexer(buf)) {
case B1:
printf ("B1!\n");
break;
case C1:
printf ("C1!\n");
break;
case D1:
printf ("D1!\n");
break;
}
}


Note that your SWITCH(S) will work only in C++ since C doesn't allow variables definition in the middle of blocks. Usually such macro uses something like:

#define BEGIN_SEPARATION(S, T) { const char *_tail = (T), *_sep = (S); \
size_t _sep_len = strlen(_token); \
for(const char *_next = strstr(_tail); _tail != NULL; _tail = (_next != NULL) ? _next + _sep_len : NULL) {
#define SEPARATION_TOKEN() (_next == NULL ? strdup(_tail) : strndup(_tail, _next - _tail))
#define END_SEPARATION } }


And as far as I know it's common practice to wrap macro arguments (i.e _S = (S);).

-
The code will compile just fine in standard C, which allows variable definitions anywhere. To me, it sounds like you are following the 23 year old, obsolete C90 standard. –  Lundin Jan 11 '13 at 7:43

I do like the idea but I'm not sure to really get the benefit of your code compared to (my C is a bit rusty so the code might not be correct) :

    if (stdcmp(buf,"abcdef") == 0) {
printf ("B1!\n");
if (stdcmp(buf,"ghijkl") == 0) {
printf ("C1!\n");
} else {
printf ("D1!\n");
}


Indeed, the code doesn't really seem easier to read but it's definitly harder to check when something wrong's happening. On top of that, the way you use brackets impose the fact that we have one BREAK per CASE/DEFAULT and this limits the expressiveness of your syntax. For instance, we don't have the following cool structures possible with a real switch :

// The fall-through (1)
case 0:
case 1: do_stuff_for1_and_2(); break

// The fall-through (1)
case 0: do_stuff_for_only1();
case 1: do_stuff_for1_and_2(); break


or

// The early return
case 0 : return FOO;
case 1 : return BAR;
default: return FOO_BAR;


or

// The normal-case with no break in the default
case 0: do_stuff(); break;
default: do_nothing();


I'm not quite sure about the kind of feedback you expected so I hope this answers your question.

As a side note, I'd choose a different variable name, maybe using __COUNTER__ (I've never used it) to generate unique name if you ever plan to nest the switches.

-

As others have pointed, out never try to re-invent the C language. It makes the code less readable and more error-prone, for no obvious benefits. Function-like macros in general are incredibly error-prone and dangerous (and a pain to debug), they should be avoided.

If you for some reason need to use function-like macros, you need to make them safe, properly encapsulate them with braces and parenthesis.

In addition, doing strcmp after strcmp in sequence like this, is very slow and inefficient, growing more inefficient with each "case" you add. This is unacceptable if program speed and random access are important.

So as for code review, I'd strongly recommend to forget this whole program as quickly as possible, nothing good will come out of it.

The proper way to write an algorithm that stores unknown, initially unsorted and completely random input strings, is to use a hash table.

-

I'd add another vote for "don't do this", you (or another) will suffer having to reread/debug it.

I'd try something along the lines of:

char* options[] = { "abcdef", "ghijkl", /* etc */ NULL };
switch ( index_of( options, buf ) )
{
case 0: // abcdef
break;
case 1: // ghijkl
break;
/* etc */
}


Which is just another version of ony's answer. Slightly simpler (maybe), but either version will need keeping in step.

-