views:

265

answers:

4

I'm adding coroutine support to an interpreter I'm writing and I'd like to do something like the following:

typedef enum {
    bar_stuff,
    bar_other
    } Bar;

typedef enum {
    foo_error=-1,
    foo_none=0,
    foo_again
    } Foo_state;

Foo_state do_foo(Bar bar,Foo_state foo)
    {
    switch(foo)
        {
        case foo_none: //start
        switch(bar)
            {
            case bar_stuff:
                //do stuff
                return foo_none;
            case bar_other:
                //do other stuff
                return foo_again;
                case foo_again: //!! this doesn't work
                    /* edit: this is supposed to be a case of
                     *     switch(foo), not switch(bar)
                     */
                //do more other stuff
                return foo_none;
            default:
                //stuff
                return foo_none;
            }
        default:
            //fail
            return foo_error;
        }
    }

Obviously this dosn't work (I get duplicate case value, alternative is probably undefined behavior/segfault). I could write switch(bar) as an if/else if/else chain, but I was hoping there was a better way.

I'm using gcc if that makes a difference.

Edit:

The following would work, but would be a PITA to maintain:

Foo_state do_foo2(Bar bar,Foo_state foo)
    {
    switch(foo)
        {
        case foo_none:  goto case_foo_none;
        case foo_again: goto case_foo_again;
        default:
            //fail
            return foo_error;
        }
    case_foo_none: //start
    switch(bar)
        {
        case bar_stuff:
            //do stuff
            return foo_none;
        case bar_other:
            //do other stuff
            return foo_again;
            case_foo_again:
            //do more other stuff
            return foo_none;
        default:
            //stuff
            return foo_none;
        }
    }

Edit 2:

Well, this doesn't seem to be yielding the aforementioned 'better way', so I'd like know if anyone foresees a problem with writing it like this:

Foo_state do_foo3(Bar bar,Foo_state foo)
    {
    switch(foo)
        {
        case foo_none: //start
        if(bar == bar_stuff)
            {
            printf("do stuff\n");
            return foo_none;
            }
        else if(bar == bar_other)
            {
            printf("do other stuff\n");
            return foo_again;
            case foo_again: //continue
            printf("do more other stuff\n");
            return foo_none;
            }
        else
            {
            printf("stuff\n");
            return foo_none;
            }
        default:
            //fail
            return foo_error;
        }
    }

The problem I see with this is missing a bar_* value (since there are several functions like this, and some of the enums have dozens of values), but I suppose a test script for that should work...

+2  A: 

Hm ... use an equivalent of a Karnaugh Map to simplify the logic and do it all with

if (cond1 && cond2) {
  doX();
  return;
}
if (cond3 && cond4) {
  doY();
  return;
}
// Sometimes you can take shortcuts
if (cond5) {
   doZ();
} else {
   doW();
}
return;

This code is readable. Nested things are best avoided if possible.

Checking the simplest conditions first will make the function simpler.

IN YOUR CASE START WITH:

Foo_state do_foo2(Bar bar,Foo_state foo) {
  if (foo != foo_none && foo != foo_again) {
    return foo_error;
  }
  ...

  if (foo == foo_none) {
    ...
  }
  // Implicit Else
  ...
Hamish Grubijan
That doesn't do the same thing, if `(foo==foo_again)` I want it to resume after the `return foo_again;`.
David X
@lpthnc: I agree with your approach...Sounds like a finite state machine that is being coded together using nested switch statements...but your approach is more cleaner and simpler!
tommieb75
+1 for Karnaugh Map suggestion
Adam Rosenfield
+1  A: 

A simple fix would be to change the values in the bar_ enum so that they are unique with respect to the foo_ enum. That doesn't address the fact, however, that your code is confusing. Why would you look for a foo_ value inside of the bar_ switch statement? Syntactically, it is valid (as long as the values are unique) but its poor coding.

sizzzzlerz
I should have been more clear, see edit.
David X
+2  A: 

You can also just put { } inside each case: statement
without them the whole case stack is evaluated as a single unit, so no variables can be defined within one case:

But by putting

 case blah:
 {
  // do stuff
 }
 break;

You can put anythign you want inside the case statement.

Martin Beckett
A: 

Ok, it seems that your code does this:

bar/foo   foo_none                          foo_again                           other
bar_stuff   doStuff, return foo_none          do more other stuff, return foo_none  return foo_error
bar_other   do other stuff, return foo_again  do more other stuff, return foo_none  return foo_error
other       stuff, return foo_none            do more other stuff, return foo_none return foo_error

This is what I meant by a Karnaugh Map. Now, here is the simplest implementation:

Foo_state do_foo2(Bar bar,Foo_state foo) {
  if (foo == foo_again) {
    // do more stuff
    return foo_none;
  }
  if (foo != foo_none) { // other
    return foo_error;
  }
  // foo_none
  if (bar == bar_stuff) {
    // do stuff
    return foo_none;
  }
  if (bar == bar_other) {
    // do other stuff
    return foo_again;
  }
  // At this point bar = other

  // stuff
  return foo_none;
}

I believe this does the same thing as your code, but does not use switches and gotos. You can fill out a table with the result, and you can also put both implementations through a unit tests to make sure they do the same thing for all inputs.

Hamish Grubijan