Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
#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;
    }
}
share|improve this question
4  
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
add comment

4 Answers 4

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

share|improve this answer
    
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
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.