A small GOTO text adventure game

Question

EDIT_START: I want to thank all people for giving me such good answers! It's hard for me to choose any answer over another, because I see that all of your answers are valid and good in their own perspective. I want to clarify my own question. My question is not "How do I not use GOTO?", but my question is "How do I use GOTO in a better way?". This implicates that I want to use GOTO for program room/state transition at all costs. This is for educational purpose and for discovering the limits of C. I will give out a bounty as soon as possible to my question, to give back a reward. Anyway thank you all! I'll place a LABEL for ya all in my program ;-) EDIT_END:

I was discussing with someone about using GOTO at stackoverflow. May someone teach me some hidden tricks in using GOTO? Do you have some suggestions for improvement? You may enjoy my little adventure game, give it a try. ^^

PS play the game before you read the source, otherwise you get spoiled

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

enum _directions{
    DIR_0 =    0b0000,
    DIR_E =    0b0001,
    DIR_W =    0b0010,
    DIR_WE =   0b0011,
    DIR_S =    0b0100,
    DIR_SE =   0b0101,
    DIR_SW =   0b0110,
    DIR_SWE =  0b0111,
    DIR_N =    0b1000,
    DIR_NE =   0b1001,
    DIR_NW =   0b1010,
    DIR_NWE =  0b1011,
    DIR_NS =   0b1100,
    DIR_NSE =  0b1101,
    DIR_NSW =  0b1110,
    DIR_NSWE = 0b1111
} DIRECTIONS;

void giveline(){
    printf("--------------------------------------------------------------------------------\n");
}

void where(int room, unsigned char dir){
    printf("\nYou are in room %i. Where do you want GOTO?\n", room);
    if(dir & 8) printf("NORTH: W\n");
    else printf(".\n");
    if(dir & 4) printf("SOUTH: S\n");
    else printf(".\n");
    if(dir & 2) printf("WEST:  A\n");
    else printf(".\n");
    if(dir & 1) printf("EAST:  D\n");
    else printf(".\n");
}

char getdir(){
    char c = getchar();
    switch(c){
        case 'w' :
        case 'W' :
            return 'N';
        case 's' :
        case 'S' :
            return 'S';
        case 'a' :
        case 'A' :
            return 'W';
        case 'd' :
        case 'D' :
            return 'E';
        case '\e' :
            return 0;
    }
    return -1;
}

int main(int argc, char *argv[]){
    START:
    printf("THE EVIL GOTO DUNGEON\n");
    printf("---------------------\n");
    printf("\nPress a direction key \"W, A, S, D\" followed with 'ENTER' for moving.\n\n");
    char dir = -1;
        
    ROOM1:
    giveline();
    printf("Somehow you've managed to wake up at this place. You see a LABEL on the wall.\n");
    printf("\"Do you know what's more evil than an EVIL GOTO DUNGEON?\"\n");
    printf("You're wondering what this cryptic message means.\n");
    where(1, DIR_SE);
    do{
        dir = getdir();
        if(dir == 'S') goto ROOM4;
        if(dir == 'E') goto ROOM2;
    }while(dir);
    goto END;
    
    ROOM2:
    giveline();
    printf("Besides another LABEL, this room is empty.\n");
    printf("\"Let's play a game!\"\n");
    where(2, DIR_W);
    do{
        dir = getdir();
        if(dir == 'W') goto ROOM1;
    }while(dir);
    goto END;
    
    ROOM3:
    giveline();
    printf("Man, dead ends are boring.\n");
    printf("Why can't I escape this nightmare?\n");
    where(3, DIR_S);
    do{
        dir = getdir();
        if(dir == 'S') goto ROOM6;
    }while(dir);
    goto END;
    
    ROOM4:
    giveline();
    printf("Is this a real place, or just fantasy?\n");
    printf("\"All good things come in three GOTOs.\"\n");
    where(4, DIR_NSE);
    do{
        dir = getdir();
        if(dir == 'N') goto ROOM1;
        if(dir == 'S') goto ROOM7;
        if(dir == 'E') goto ROOM5;
    }while(dir);
    goto END;
    
    ROOM5:
    giveline();
    printf("This is a big river crossing. I guess I need to JUMP.\n");
    where(5, DIR_SWE);
    do{
        dir = getdir();
        if(dir == 'S') goto ROOM8;
        if(dir == 'W') goto ROOM4;
        if(dir == 'E') goto ROOM6;
    }while(dir);
    goto END;
    
    ROOM6:
    giveline();
    printf("This place doesn't look very promising.\n");
    where(6, DIR_NSW);
    do{
        dir = getdir();
        if(dir == 'N') goto ROOM3;
        if(dir == 'S') goto ROOM9;
        if(dir == 'W') goto ROOM5;
    }while(dir);
    goto END;
    
    ROOM7:
    giveline();
    printf("\"Give a man a LOOP and you feed him FOR a WHILE;\n");
    printf(" teach a man a GOTO and you feed him for a RUNTIME.\"\n");
    where(7, DIR_NE);
    do{
        dir = getdir();
        if(dir == 'N') goto ROOM4;
        if(dir == 'E') goto ROOM8;
    }while(dir);
    goto END;
    
    ROOM8:
    giveline();
    printf("This looks like an endless LOOP of rooms.\n");
    where(8, DIR_NW);
    do{
        dir = getdir();
        if(dir == 'N') goto ROOM5;
        if(dir == 'W') goto ROOM7;
    }while(dir);
    goto END;
    
    ROOM9:
    giveline();
    printf("You've found your old friend Domino. He doesn't looks scared, like you do.\n");
    printf("\n\"Listen my friend,\n");
    printf(" If you want to escape this place, you need to find the ESCAPE KEY.\"\n");
    printf("\nWhat does this mean?\n");
    where(9, DIR_N);
    do{
        dir = getdir();
        if(dir == 'N') goto ROOM6;
    }while(dir);
    goto END;
    
    printf("You never saw me.\n");
    
    END:
    giveline();
    printf("The End\n");
    return 0;
}

Answer 1

May someone teach me some hidden tricks in using GOTO?

goto is kind of a one trick pony. But is part of a family of control transfer constructs. All the looping and decision structures are in a sense a refined or specialized application of a goto. So from that pov, using goto is advisable only if it cannot be done with any of the "normal" control structures like if, while, for etc.

The next level up is function calls. A function call is a super-powered goto. Not only can you jump and execute a named piece of code, but you can also jump back right where you came from. Plus you can pass arguments and return a value, if desired.

Another level up is making use of function pointers. A pointer to a function can be saved in a variable or an array. It can be passed to and returned from functions. For a state machine like in the question, I'd be very tempted to organize the rooms into an array of function pointers. But I'd probably use a lot of macros, too.

The next level up from functions is setjmp/longjmp. These let you jump back across several levels of the call stack. It's sometimes useful to have a setjmp call in the main loop or initialization of the program and then the program can restart or bail-out if it runs into certain recoverable errors.

I suppose the next level up might be signal handlers and/or forking off child processes. Or maybe loading a dynamic library.

Answer 2

The goto debate is ancient, from the year 1966 when Edgar Dijkstra came up with a famous paper called "Go To Statement Considered Harmful". This was controversial and the debate is still going on to this day. Still, most of his conclusions are also valid to this day and most uses of goto is considered harmful spaghetti programming.

However, there's a broad consensus that some uses of goto are acceptable. Specifically:

• goto should only be used to jump downwards, never upwards.
• goto should only be used for the sake of error handling and centralized clean-up at the end of a function.

This is an old "design pattern" that I believe/fear originates from BASIC which had "on error goto..." as it's preferred way of error handling. Basically it's only considered OK to use goto like in this made-up example:

status_t func (void)
{
  status_t status = OK;

  stuff_t* stuff = allocate_stuff();
  ...

  while(something)
  {
    while(something_else)
    {
      status = get_status();
      if(status == ERROR)
      {
        goto error_handler; // stop execution and break out of nested loops/statements
      }
    }
  }
 
 goto ok;

 error_handler:
   handle_error(status);

 ok:
   cleanup(stuff);
   return status;
}

Use of goto as in the above example is considered acceptable. There are two obvious benefits: clean way of breaking out of nested statements and centralized error handling and clean-up at the end of the function, avoiding code repetition.

Still it is possible to write the same thing with return and a wrapper function, which I personally find much cleaner and avoids the "goto considered harmful" debate:

static status_t internal_func (stuff_t* stuff)
{
  status_t status = OK;
  
  ...
  
  while(something)
  {
    while(something_else)
    {
      status = get_status();
      if(status == ERROR)
      {
        return status;
      }
    }
  }

  return status;
}

status_t func (void)
{
  status_t status;

  stuff_t* stuff = allocate_stuff();  

  status = internal_func(stuff);

  if(status != OK)
  {
    handle_error(status);
  }

  cleanup(stuff);
  return status;
}

EDIT:

I posted a separate lengthy answer here regarding everything not goto-related. Including a proposal for how to rewrite the whole program using a proper state machine design.

Answer 3

The code you've written is more or less a state machine, written the way that one might be constructed in assembly language. A technique like that technically works, but it doesn't scale well and you can wind up with problems that are extremely hard to debug. Your code only needs a small tweak to use the more traditional C-language way to implement a state machine, which is easier to read, maintain, and debug.

int main(int argc, char *argv[])
{
    int state = START;
    char dir = -1;
    while(1)
    {
        switch (state)
        {
            case START:
                printf("THE EVIL GOTO DUNGEON\n");
                printf("---------------------\n");
                printf("\nPress a direction key \"W, A, S, D\" followed with 'ENTER' for moving.\n\n");
                state = ROOM1;
                break;
                
            case ROOM1:
                giveline();
                printf("Somehow you've managed to wake up at this place. You see a LABEL on the wall.\n");
                printf("\"Do you know what's more evil than an EVIL GOTO DUNGEON?\"\n");
                printf("You're wondering what this cryptic message means.\n");
                where(1, DIR_SE);
                do{
                    dir = getdir();
                    if(dir == 'S') { state = ROOM4; break; }
                    if(dir == 'E') { state = ROOM2; break; }
                }while(dir);
                break;
            
            case ROOM2:
                giveline();
                printf("Besides another LABEL, this room is empty.\n");
                printf("\"Let's play a game!\"\n");
                where(2, DIR_W);
                do{
                    dir = getdir();
                    if(dir == 'W') { state = ROOM1; break; }
                }while(dir);
                break;
            
            ...
            
            case END:
                giveline();
                printf("The End\n");
                return 0;
        }
    }
}

The code is mostly the same as before, with only a few small tweaks:

• Added the enclosing loop and switch
• Changed labels from ROOMX: to case ROOMX:
• Changed jumps from goto ROOMX; to state = ROOMX; break;
• Defined constants for START, ROOMX, etc (not shown)

Structuring your code in this way makes it more readable and avoids a lot of problems that goto spaghetti can have. It's a lot easier to ensure that you don't unintentionally fall through from one room's code into the next one (if you somehow bypass the code that sets a new state, you simply stay in the same room and try again). You also avoid many limitations of goto, like the inability to "jump over" the declaration of a variable-length array (see example 2 in section 6.8.6.1 of the C99 language spec). You can also add an explicit default case to intelligently handle any unexpected or erroneous room selections.

This type of structure also opens up all sorts of avenues for improvement. You could take the contents of each case and wrap it up in a function, and each case could be simplified to case ROOMX: state = do_roomx(); break;. With each room's code encapsulated, you could unit test rooms individually.

You could also notice that each room's code follows a predictable sequence (giveline() -> print description -> where() -> read input -> select next room) and write a generic function do_room(struct room_data* room) that could handle any arbitrary room. You'd then create a data structure struct room_data that holds all the information needed for each room (description text, movement directions, where each exit leads, etc). This would be more similar to how a game engine works. Your code would become shorter and more generic, and each individual room would be implemented as data instead of code. You could even store the room data in an external file, and then you'd have a generic game engine that you wouldn't have to recompile every time you wanted to modify your maze.

---

Asking "How do I use GOTO in a better way?" is akin to asking "How can I punch myself in the face in a better way?" The answer is: You don't. Using goto like you're doing is incompatible with any definition of the word "better" that I'm aware of. You're taking a construct that C handles natively (a switch block) and have re-implemented it using explicit jumps. You get less functionality and more potential problems. The only way to approach "better" is to drop the unnecessary gotos.

Remember that the C language is just a thin, portable veneer on top of assembly language. goto is a wrapper around your CPU's "jump" instruction. None of the CPUs that I know of have instructions comparable to things like switch or for. These are syntactic sugar that the compiler rewrites for you into a sequence powered by "jump" instructions. For example, a simple loop like this:

for (i = 0; i < limit; i++)
{
    ... code ...
}

is treated as if it was written like this:

    i = 0;
LOOP_START:
    if (!(i < limit))
        goto LOOP_END;
    ... code ...
LOOP_CONTINUE:
    i++;
    goto LOOP_START;
LOOP_END:

A continue statement would be equivalent to goto LOOP_CONTINUE, and a break statement would be equivalent to goto LOOP_END.

A switch block is implemented similarly. Each case is a block of code with a label, and the switch jumps to a label based on the input value. break jumps to the end. This is generally similar to the way that you wrote your code. The key difference is that a switch block doesn't jump directly between cases. If you want to execute more than one case, you use a loop to run the switch block more than once.

Ultimately, the switch version and the goto version likely look almost identical once they're compiled. When you use switch, you give the compiler the chance to avoid certain problems such as ensuring that you're not jumping into the scope of a local variable while skipping over its initializer. When you write the goto-based version, the compiler will compile your code as written and trust that you know what you're doing. If you insist on using goto explicitly, you'll end up running into the sort of problems that led people to invent things like switch in the first place. When using goto "at all costs", those costs are frequently a program that behaves inconsistently and unpredictably. If you're looking for advice on how to program poorly, you're in the wrong place.

Answer 4

Lack of Error Checking on User Input

The function getdir() should check for valid input, perhaps is should receive an array of valid directions. When an invalid direction is entered there should be a message to the user that the input was invalid.

DRY Code

The use of goto is forcing you to repeat code that shouldn't be repeated such as

    where(2, DIR_W);
    do {
        dir = getdir();
        if (dir == 'W') goto ROOM1;
    } while (dir);
    goto END;

Spaghetti Code

The entire program seems to be an example of how to write Spaghetti Code which is unstructured code that is very difficult to write, debug and maintain.

The code would actually be smaller and more understandable if it was structured and used while loops or for loops.

Using Binary in ENUM

The more characters you type the easier it is to make a mistake. Since Bits are important in the enum I would suggest using Octal or Hexadecimal, preferably Hexadecimal. Each enum could then be defined using one character.

Rather than using magic numbers in the code, define masks.

Answer 5

I'm looking at the goto usage in this program, and I'm going to be controversal here and say it's not that bad. I've seen much worse. Here is a list of things this program does not do.

• This program never uses goto to go around a variable initialization except where the value is clearly thrown out.
• This program never does a goto into an inner block. All operational goto statements go to the outermost block.
• This program does not in fact have haphazard use of goto. All the goto operations center around one operation. (Looks like two, but the strings indicate it is in fact one.)

I've seen the recommendation to build a loop-switch for the state machine. I've done that before. I'm tired of debugging it. (Oh look there's even an answer.) It's harder to do state machines in loop-switch than it is to do them in goto statements.

But looking at the code it is clearly not a tight loop. Nobody cares about microseconds of performance around the loop. We can do better these days. To get back to the ease of goto while still maintaining the scope niceties we can do function state machines. This is higher up the learning ladder, and not everybody gets here early.

It looks as follows: The casts are necessary as the type simply can't be expressed in the type system.

typedef void (*funcptr)(void);
typedef funcptr (*ptrfuncptr)();

int main(int argc, char *argv[])
{
        ptrfuncptr state = START;
        while (state)
            state = (pfuncptr)state();
}

funcptr START()
{
    printf("THE EVIL GOTO DUNGEON\n");
    printf("---------------------\n");
    printf("\nPress a direction key \"W, A, S, D\" followed with 'ENTER' for moving.\n\n");
    return (funcptr)ROOM1;
}

funcptr ROOM1()
{
    giveline();
    printf("Somehow you've managed to wake up at this place. You see a LABEL on the wall.\n");
    printf("\"Do you know what's more evil than an EVIL GOTO DUNGEON?\"\n");
    printf("You're wondering what this cryptic message means.\n");
    where(1, DIR_SE);
    do{
        dir = getdir();
        if(dir == 'S') return (funcptr)ROOM4;
        if(dir == 'E') return (funcptr)ROOM2;
    }while(dir);
    return NULL;
}
// ...

You should not call these ROOM1, ROOM2, ROOM3, etc. If you did that, an array would have suited you better. You should give these all descriptive names. Furthermore, you should change where to take a const char * rather than an int as its first argument.

If you want to win the goto debate, use a lexer as an example. There's just not much else anymore where it's the best way anymore. https://gist.github.com/tomtheisen/cf6afee1210b9625181bb3c23ea0f204#file-csvreader-cs

Answer 6

Quuxplusone's answer briefly mentioned making this data-driven, and I've fleshed out the idea here.

The key realization is that each room varies based only on a few pieces of information: a numeric label, a description string, a set of valid directions, and the label of the room that each valid direction leads to. The original implementation deals with this information using similar code repeated in every 'block'. Implemented that way, making a uniform change requires many modifications in many places, which is error-prone.

Since all of the per-room behavior only depends on a few pieces of information, there is no need to repeat slightly modified code (whether in labeled blocks referenced by goto, cases in a switch statement, or functions referenced using function pointers), which leads to a better realization of the Don't Repeat Yourself principle. Instead of repeated code, you can have a relatively small amount of unique code that acts as required based on external data.

To implement a data-driven approach, some kind of data structure can be used to store the relevant details about each room. Since a room may theoretically have up to 4 neighbors, an easy way to store the neighbors would be as an array of 4 room ids. If we define a constant to represent an invalid room id, the room id stored in each entry directly indicates whether that direction is valid, so there is no need to store the list of valid directions separately.

Since there is a data structure representing a room, we can pass the current room's structure to the various functions that need information about it (where in the original code, and an improved version of getdir that includes looping on invalid inputs) instead of passing the various values separately. This allows some of the benefits of encapsulation, in that future versions of the function can use different fields of the room information structure without requiring every invocation to change.

#include <stddef.h>
#include <limits.h>
#include <stdint.h>
#include <inttypes.h>
#include <stdio.h>
#include <assert.h>

typedef uint_least32_t ROOM_ID;
#define PRI_ROOM_ID PRIuLEAST32
#define INVALID_ROOM_ID UINT_LEAST32_MAX

typedef enum {
    NORTH = 0, //The first 4 values are used as indices, so the exact value matters
    EAST = 1,
    WEST = 2,
    SOUTH = 3,
    ESCAPE_DIRECTION //This is not used as an index, so its value doesn't matter
} DIRECTION;

typedef struct {
    ROOM_ID id;
    const char *description;
    ROOM_ID target_ids[4];
} ROOM;

const ROOM all_rooms[] = {
    {1, "Somehow you've managed to wake up at this place. [...]", {INVALID_ROOM_ID, 2, INVALID_ROOM_ID, 4}},
    //...
};

const ROOM *find_room(ROOM_ID room_id)
{
    size_t i;
    for(i = 0; i < sizeof(all_rooms)/sizeof(all_rooms[0]); ++i)
    {
        if(all_rooms[i].id == room_id)
        {
            return &all_rooms[i];
        }
    }
    return NULL;
}

//Precondition: room is not NULL
void display_where(const ROOM *room)
{
    const struct {
        DIRECTION dir;
        const char *str;
    } descriptions[4] = {{NORTH, "NORTH: W"}, {SOUTH, "SOUTH: S"}, {WEST, "WEST: A"}, {EAST, "EAST: D"}};
    size_t i;
    assert(room != NULL);
    printf("\nYou are in room %" PRI_ROOM_ID ". Where do you want GOTO?\n", room->id);
    for(i = 0; i < 4; ++i)
    {
        if(room->target_ids[descriptions[i].dir] != INVALID_ROOM_ID)
        {
            puts(descriptions[i].str);
        }
        else
        {
            puts(".");
        }
    }
}

//Precondition: room is not NULL
DIRECTION get_dir(const ROOM *room)
{
    while(1)
    {
        int c = getchar();
        switch(c){
            case 'w' :
            case 'W' :
                if(room->target_ids[NORTH] != INVALID_ROOM_ID)
                {
                    return NORTH;
                }
                break;
            case 's' :
            case 'S' :
                if(room->target_ids[SOUTH] != INVALID_ROOM_ID)
                {
                    return SOUTH;
                }
                break;
            //...
            case '\e' :
                return ESCAPE_DIRECTION;
        }
    }
}

int main(void)
{
    const ROOM_ID FIRST_ROOM_ID = 1;
    const ROOM *room = NULL;
    printf("THE EVIL GOTO DUNGEON\n");
    printf("---------------------\n");
    printf("\nPress a direction key \"W, A, S, D\" followed with 'ENTER' for moving.\n\n");

    room = find_room(FIRST_ROOM_ID);
    while(room)
    {
        DIRECTION dir;
        puts("--------------------------------------------------------------------------------");
        puts(room->description);
        display_where(room);
        dir = get_dir(room);
        if(dir == ESCAPE_DIRECTION)
        {
            break;
        }
        else
        {
            room = find_room(room->target_ids[dir]);
        }
    }
}

I'm sure many improvements upon the above are possible, but I think this sufficiently demonstrates the basic idea of data-driven code.

---

Comments on aspects of the original code besides code repetition:

Defining every combination of values in a bitmask is unnecessary, since a primary benefit of using bitflags in the first place is that they can be manipulated using bitwise operators. For example, in room 6, instead of using where(6, DIR_NSW) you could use where(6, DIR_N | DIR_S | DIR_W). Following this practice overall would mean you can get rid of 11 constant definitions and you wouldn't need to remember which order the flags are listed in the combination values.

Related to using bitfields, your code would be more clear if you used your constants instead of magic numbers. For example, in your where function you could use if(dir & DIR_N) instead of if(dir & 8). To get in a habit that is more generally applicable (even when flags are multi-bit values), you may want to standardize on something like if((dir & DIR_N) == DIR_N).

If you're going to keep your code structure, you could improve it by making getdir accept a description of valid directions as a parameter and have it internally loop until a valid direction is selected. With that change, you could remove all the do/while loops surrounding each invocation (but not the loop body - you still want to actually act on the input).

You could also somewhat reduce the code repetition by creating a function that handles displaying the room and getting the next direction. A signature like DIRECTIONS handle_room(int id, const char *description, DIRECTIONS dirs) might work. Combined with the previous suggestions, each room's code could be made much much shorter, with only the branching repeated.

    ROOM6:
    dir = handle_room(6, "This place doesn't look very promising.\n", DIR_N | DIR_S | DIR_W);
    if(dir == 'N') goto ROOM3;
    if(dir == 'S') goto ROOM9;
    if(dir == 'W') goto ROOM5;
    goto END;

Answer 7

My previous review focused solely on the use of goto. Since then, various answers about state machines and code repetition have popped up. So here is another answer regarding everything else except goto & suggestions for how you could rewrite this better.

C Language

• Binary literals are not standard C and should therefore be avoided. I can see why you added them as self-documenting code to make sense of the bit masking in the where function. But it probably doesn't make much sense to use bit-fields for this program to begin with - plain numbers might be more readable.
• The \e escape sequence is not standard C and therefore non-portable. Ironically, escape key has no standardized escape sequence.
• Never use void func (); empty parenthesis. This is obsolete style in C and means "accept any parameter", which makes such functions less type safe. Instead use void func (void);. (C and C++ are different here.)
• Assinging -1 to a char is non-portable, since the char type has implementation-defined signedness and might be unsigned on some systems. As a rule of thumb, never use char for anything but text and particularly never use it for any form of arithmetic.
• Unintuitively, getchar returns an int, not a char. Because it may return EOF. Make a habit of always using an int for the result, even though I don't think it matters in this case.

Style/best practices

• Your enum should be changed to typedef enum { ... } typename; and then use variables of type typename when you refer to the enumeration constants, not some int or char etc.

• Use consistent coding style with indention and line breaks. It is very hard to read code such as this:

  if(dir & 8) printf("NORTH: W\n");
  else printf(".\n");
  if(dir & 4) printf("SOUTH: S\n");
  else printf(".\n");
  

  Instead, do this (optionally with { }) :

  if(dir & 8) 
      printf("NORTH: W\n");
  else 
      printf(".\n");
  

• Ideally all C programs exist in multiple files. In which case it is custom to make local functions static.

Program design

• The "spaghetti" in this program isn't as much the fault of goto as the fault of decentralizing room changes (state machine changes) and placing them all over the code. Switching out goto for a state/room variable doesn't fix this, it's "stateghetti" instead of "spaghetti", just a different flavour of the same thing.

  My recommended practice for state machines is to centralize all decision making to one single place. Preferably together with error handling. The ideal state machine looks like this:

    for(;;)
    {
      status = state_machine[state]();
      state = decision_maker(status);
    }
  

  Here the state application logic has been separated from state transition logic. So we don't have to dig through all individual states to figure out which one that caused a state change to what. Everything is centralized inside the "decision_maker", including optional error handling.

  I'll make an example for how this can be applied to your code at the bottom of this post.

• As pointed out in other reviews, this program suffers from a lot of code repetition, which is always a bad thing, particularly during maintenance. This can be fixed by placing all repeated code in the caller and only place room-specific things inside the room logic code. If we rewrite this to a proper state machine, we can fix that problem at the same time.

After a complete make-over, I came up with a main looking like this:

int main (void)
{
    printf("THE EVIL GOTO DUNGEON\n");
    printf("---------------------\n");
    printf("\nPress a direction key \"W, A, S, D\" followed with 'ENTER' for moving.\n\n");

    int current_room = 1;

    for(;;)
    {
      giveline();
      ROOM[current_room]();
    
      int next_room;
      do
      {
        next_room = get_next_room(getdir(), current_room);
      } while(next_room == 0);
      current_room = next_room;
    }    
    
    printf("You never saw me.\n");

    giveline();
    printf("The End\n");
    return 0;
}

Unfortunately, this version lacks any means to escape the dungeon, since my compiler didn't support \e. Otherwise the for(;;) should be replaced with while(stuck_in_dungeon).

The key features here is the ROOM function pointer array, which is a state machine. All decision making has been given to a function called get_next_room, which is the only function that knows how rooms are connected, and which rooms you have access to based on the current one. It's essentially just one big table based on switch:

static int get_next_room (direction dir, int current_room)
{
  switch(current_room)
  {
    case 1:
      if(dir == 'S') return 4;
      if(dir == 'E') return 2;
      break;
    case 2:
      if(dir == 'W') return 1;
      break;

    ...

Complete code follows. Only briefly tested, but the key here is to look at the state machine logic instead of spaghetti.

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

#define ROOMS 9

typedef enum {
    DIR_0,
    DIR_E,
    DIR_W,
    DIR_WE,
    DIR_S,
    DIR_SE,
    DIR_SW,
    DIR_SWE,
    DIR_N,
    DIR_NE,
    DIR_NW,
    DIR_NWE,
    DIR_NS,
    DIR_NSE,
    DIR_NSW,
    DIR_NSWE,
} direction;


typedef void room_func (void);

static void room1 (void);
static void room2 (void);
static void room3 (void);
static void room4 (void);
static void room5 (void);
static void room6 (void);
static void room7 (void);
static void room8 (void);
static void room9 (void);

room_func* const ROOM [ROOMS+1] = 
{
  NULL, // invalid room
  room1,
  room2,
  room3,
  room4,
  room5,
  room6,
  room7,
  room8,
  room9,
};

static int get_next_room (direction dir, int current_room);

static void giveline(void);
static void where(int room, direction dir);
static char getdir (void);

int main (void)
{
    printf("THE EVIL GOTO DUNGEON\n");
    printf("---------------------\n");
    printf("\nPress a direction key \"W, A, S, D\" followed with 'ENTER' for moving.\n\n");

    int current_room = 1;

    for(;;)
    {
      giveline();
      ROOM[current_room]();
    
      int next_room;
      do
      {
        next_room = get_next_room(getdir(), current_room);
      } while(next_room == 0);
      current_room = next_room;
    }    
    
    printf("You never saw me.\n");

    giveline();
    printf("The End\n");
    return 0;
}


static void room1 (void)
{
    printf("Somehow you've managed to wake up at this place. You see a LABEL on the wall.\n");
    printf("\"Do you know what's more evil than an EVIL GOTO DUNGEON?\"\n");
    printf("You're wondering what this cryptic message means.\n");
    where(1, DIR_SE);
}

static void room2 (void)
{
    printf("Besides another LABEL, this room is empty.\n");
    printf("\"Let's play a game!\"\n");
    where(2, DIR_W);
}

static void room3 (void)
{
    printf("Man, dead ends are boring.\n");
    printf("Why can't I escape this nightmare?\n");
    where(3, DIR_S);
}


static void room4 (void)
{
    printf("Is this a real place, or just fantasy?\n");
    printf("\"All good things come in three GOTOs.\"\n");
    where(4, DIR_NSE);
}

static void room5 (void)
{
    printf("This is a big river crossing. I guess I need to JUMP.\n");
    where(5, DIR_SWE);
}

static void room6 (void)
{
    printf("This place doesn't look very promising.\n");
    where(6, DIR_NSW);
}

static void room7 (void)
{
    printf("\"Give a man a LOOP and you feed him FOR a WHILE;\n");
    printf(" teach a man a GOTO and you feed him for a RUNTIME.\"\n");
    where(7, DIR_NE);
}

static void room8 (void)
{
    printf("This looks like an endless LOOP of rooms.\n");
    where(8, DIR_NW);
}

static void room9 (void)
{
    printf("You've found your old friend Domino. He doesn't look scared, like you do.\n");
    printf("\n\"Listen my friend,\n");
    printf(" If you want to escape this place, you need to find the escape sequence.\n");
    printf("\nWhat does this mean? There no standardized escape sequence for the ESCAPE KEY!\n");
    printf("\nAAAAAH!!!\n");
    where(9, DIR_N);
}

static int get_next_room (direction dir, int current_room)
{
  switch(current_room)
  {
    case 1:
      if(dir == 'S') return 4;
      if(dir == 'E') return 2;
      break;
    case 2:
      if(dir == 'W') return 1;
      break;
    case 3:
      if(dir == 'S') return 6;
      break;
    case 4:
      if(dir == 'N') return 1;
      if(dir == 'S') return 7;
      if(dir == 'E') return 5;
      break;
    case 5:
      if(dir == 'S') return 8;
      if(dir == 'W') return 4;
      if(dir == 'E') return 6;
      break;
    case 6:
      if(dir == 'N') return 3;
      if(dir == 'S') return 9;
      if(dir == 'W') return 5;
      break;
    case 7:
      if(dir == 'N') return 4;
      if(dir == 'E') return 8;
      break;
    case 8:
      if(dir == 'N') return 5;
      if(dir == 'W') return 7;
      break;
    case 9:
      if(dir == 'N') return 6;
      break;
  }
  return 0;
}

static void giveline(void){
    printf("--------------------------------------------------------------------------------\n");
}

static void where(int room, direction dir){
    printf("\nYou are in room %i. Where do you want GOTO?\n", room);
    if(dir & 8) printf("NORTH: W\n");
    else printf(".\n");
    if(dir & 4) printf("SOUTH: S\n");
    else printf(".\n");
    if(dir & 2) printf("WEST:  A\n");
    else printf(".\n");
    if(dir & 1) printf("EAST:  D\n");
    else printf(".\n");
}

static char getdir (void){
    char c = getchar();
    switch(c){
        case 'w' :
        case 'W' :
            return 'N';
        case 's' :
        case 'S' :
            return 'S';
        case 'a' :
        case 'A' :
            return 'W';
        case 'd' :
        case 'D' :
            return 'E';
    }
    return -1;
}

Unfortunately this also ruined all the goto puns :(

Answer 8

I suggest reading Donald Knuth's article Structured Programming With goto statements (1974) Originally published in Computing Surveys 6, and the second chapter of his book Literate Programming. There he makes excellent points on both sides of the goto debate.

Answer 9

I'll take an alternative tack from the other answerers: Your code organization is not bad, and all that remains to do is eliminate redundancy. Notice that every room in your game has the same basic structure:

ROOM7:
giveline();
printf("\"Give a man a LOOP and you feed him FOR a WHILE;\n");
printf(" teach a man a GOTO and you feed him for a RUNTIME.\"\n");
where(7, DIR_NE);
do{
    dir = getdir();
    if(dir == 'N') goto ROOM4;
    if(dir == 'E') goto ROOM8;
}while(dir);
goto END;

If we had a macro something like DEFINE_ROOM, we could just write

DEFINE_ROOM(7, DIR_NE,
    "\"Give a man a LOOP and you feed him FOR a WHILE;\n"
    " teach a man a GOTO and you feed him for a RUNTIME.\"\n",
    'N', ROOM4,
    'E', ROOM8
);

It's pretty complicated to write a C macro that can take an arbitrary number of room exits, so instead, I'll write separate macros for each combination of room directions.

#define DEFINE_ROOM_NE(num, desc, roomN, roomE) \
    ROOM##num: giveline(); printf(desc); \
    where(num, DIR_NE); \
    while (dir = getdir()) { \
        if (dir == 'N') goto roomN; \
        if (dir == 'E') goto roomE; \
    } \
    goto END;
#define DEFINE_ROOM_NW(num, desc, roomN, roomW) \
    ROOM##num: giveline(); printf(desc); \
    where(num, DIR_NW); \
    while (dir = getdir()) { \
        if (dir == 'N') goto roomN; \
        if (dir == 'W') goto roomW; \
    } \
    goto END;

In fact, wait a minute, we can do better! Notice that if you enter a bogus direction, you just go around the loop again. So I can add a SAME##n label to do this:

#define DEFINE_ROOM(dirs, num, desc, roomN, roomS, roomE, roomW) \
    ROOM##num: giveline(); printf(desc); \
    where(num, DIR_##dirs); \
    while (dir = getdir()) { \
        if (dir == 'N') goto roomN; \
        if (dir == 'S') goto roomS; \
        if (dir == 'E') goto roomE; \
        if (dir == 'W') goto roomW; \
        SAME##num: ; \
    } \
    goto END;
#define DEFINE_ROOM_N(n, roomN, d) DEFINE_ROOM(N, n, d, roomN, SAME##n, SAME##n, SAME##n)
#define DEFINE_ROOM_S(n, roomS, d) DEFINE_ROOM(S, n, d, SAME##n, roomS, SAME##n, SAME##n)
#define DEFINE_ROOM_E(n, roomE, d) DEFINE_ROOM(E, n, d, SAME##n, SAME##n, roomE, SAME##n)
#define DEFINE_ROOM_W(n, roomW, d) DEFINE_ROOM(W, n, d, SAME##n, SAME##n, SAME##n, roomW)
#define DEFINE_ROOM_NS(n, roomN, roomS, d) DEFINE_ROOM(NS, n, d, roomN, roomS, SAME##n, SAME##n)
[...]

And now your whole game fits in an appropriate number of lines:

DEFINE_ROOM_SE(1, ROOM4, ROOM2,
    "Somehow you've managed to wake up at this place. You see a LABEL on the wall.\n"
    "\"Do you know what's more evil than an EVIL GOTO DUNGEON?\"\n"
    "You're wondering what this cryptic message means.\n"
);
DEFINE_ROOM_W(2, ROOM1,
    "Besides another LABEL, this room is empty.\n"
);
DEFINE_ROOM_S(3, ROOM6,
    "Man, dead ends are boring.\n"
    "Why can't I escape this nightmare?\n"
);
DEFINE_ROOM_NSE(4, ROOM1, ROOM7, ROOM5,
    "Is this a real place, or just fantasy?\n"
    "\"All good things come in three GOTOs.\"\n"
);
DEFINE_ROOM_SWE(5, ROOM8, ROOM4, ROOM6,
    "This is a big river crossing. I guess I need to JUMP.\n"
);
DEFINE_ROOM_NSW(6, ROOM3, ROOM9, ROOM5,
    "This place doesn't look very promising.\n"
);
DEFINE_ROOM_NE(7, ROOM4, ROOM8,
    "\"Give a man a LOOP and you feed him FOR a WHILE;\n");
    " teach a man a GOTO and you feed him for a RUNTIME.\"\n"
);    
DEFINE_ROOM_NW(8, ROOM5, ROOM7,
    "This looks like an endless LOOP of rooms.\n"
);
DEFINE_ROOM_N(9, ROOM6,
    "You've found your old friend Domino. He doesn't looks scared, like you do.\n"
    "\n\"Listen my friend,\n"
    " If you want to escape this place, you need to find the ESCAPE KEY.\"\n"
    "\nWhat does this mean?\n"
);

I see you're intending to add more motion verbs, such as JUMP. Do you see how to fit JUMP into this pattern? How many motion verbs can you support before this one-macro-per-kind-of-room pattern starts to break down?

The next step is to stop writing macros that directly generate code and start writing macros that generate data tables, which are then passed to code that knows how to loop over the table's entries and do something for each entry (such as test, "Did the user enter the motion verb in column 1? if so, goto the room in column 2").

For a (classic!) worked example, see Donald Knuth's CWEB port of Adventure, or my own C99 port here.

Answer 10

The program you give as an example would be better-designed as a finite state machine. The states could be implemented as mutually-recursive tail calls, which a modern compiler would optimize to jump instructions, just like a goto. You could then represent the states as function pointers and eliminate the switch.

Joshua has a demonstration of code somewhat like that, and Lundin refines it, but it might be better to have each room call the next room function mutually tail-recursively, until the game is over and the last room function finally returns a value. For example, it might return the score.

An optimizing compiler will compile the tail call, since it is to a function with the same type, as an unconditional jump. That is also how it would compile a goto. Since you are no longer discombobulating the C parser by making a function return a pointer to a function of the same type, you no longer need to cast function pointers to different types. You also no longer need to have invalid null function pointers in your program that would crash it if they are ever called. Doing it this way would preserve type safety and let the compiler ensure that you are actually calling valid code.

The next step might be to observe that most or all of these rooms perform the same operations on different data, write this code as a single function that takes a data structure, and pass along the room data as a parameter of the function. This could be a pointer to static data, or an index into an array of rooms. Either is a constant-time look-up, simpler and more efficient than a switch block.

If you do need more than one type of room with different code, and you pass around both room data and a function pointer to the room code, you have reinvented polymorphic objects with data members and methods.

I’ve used a goto once myself, not counting programs I wrote in BASIC as a child. It was to break out of multiple levels of a nested loop. I considered, but rejected, refactoring those levels of the loop as a function I could return from, or creating a loop-control variable. I don’t regret it.