HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Developing a better state pattern in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
patternstatebetterdeveloping

Problem

I have created a state diagram to show the different transitions and states. I could not find many examples of the state pattern in C, so I have taken an example from a Java state pattern and tried to convert it to C.

Is there a way to create the state pattern in C? I'd like some tips or tricks in doing this better using the state pattern.

This is compiled under gcc (GCC) 4.7.2.

watch_state.h (interface)

#ifndef WATCH_STATE_H_INCLUDED
#define WATCH_STATE_H_INCLUDED

/* An incomplete type for the state representation itself */
typedef struct tag_watch_state watch_state_t;

/* Function pointer to the implementation of the interface */
typedef void (*event_start_func_f)(watch_state_t *sw);
typedef void (*event_stop_func_f)(watch_state_t *sw);
typedef void (*event_split_func_f)(watch_state_t *sw);

typedef enum states_tag states_e;
enum states_tag { STOPPED, STARTED, SPLIT };

struct tag_watch_state {
    /* Events */
    event_start_func_f start;
    event_split_func_f split;
    event_stop_func_f stop;

    /* States */
    states_e current_state;

    int time;
};

void initialize(watch_state_t *state);
char* state_to_string(states_e state);

#endif /* WATCH_STATE_H_INCLUDED */


watch_state.c (implementation of interface)

```
#include
#include "watch_state.h"
#include "started_state.h"
#include "stopped_state.h"
#include "split_state.h"

static void default_start(watch_state_t *state);
static void default_split(watch_state_t *state);
static void default_stop(watch_state_t *state);

void initialize(watch_state_t *state)
{
state->start = default_start;
state->split = default_split;
state->stop = default_stop;
state->current_state = STOPPED;
state->time = 0;
}

char* state_to_string(states_e state)
{
switch(state) {
case STOPPED:
return "STOPPED";

case STARTED:
return "STARTED";

case SPLIT:
return "SPLIT";

default:
return "UNKNOWN";
}
}

static void default_start(watch_state_t

Solution

Looking at the State Pattern on Wikipedia I'd say it's more normal to replace the entire function table at once, rather than write individual function pointers.

To this end, I would do this:

struct watch_state {
    event_start_func_f start;
    event_split_func_f split;
    event_stop_func_f stop;

    const char *name;
};

struct watch_state started  = {default_start, split, stop, "Started"};
struct watch_state stopped  = {start, split, default_stop, "Stopped"};
struct watch_state splitted = {default_start, split, stop, "Splitted"};


And then this:

struct tag_watch_state {
    /* State */
    struct watch_state *state;

    int time;
};


And implement your state switches like this:

static void start(watch_state_t *state)
{
    unsigned int secs = 5;
    unsigned int i = 0;

    /* Do some work here to simulate stop*/
    for(i = 0; i state = &started;
    state->time = 0;

    printf("[ %s ] state [ %s ] time [ %d ]\n", __func__, state->name, state->time);
}


You can then delete initialize and state_to_string (but make sure you set your initial state in the watch constructor).

Finally, and this is just advice for C code in general, I would consolidate all the functions that really form one module into one .c file, define the private types inside that, and only expose the public facing types, perhaps as abstract structs, in a corresponding header.

In this case I would expect you to have client_sw.c, sw.c and sw.h that exports only the constructor, sw_create, the methods stop_watch, start_watch, and split_watch (each of which I would rename according to the sw_* theme), and the destructor sw_destroy. You would also need to expose the abstract type struct tag_watch_state, but not the internal definition.

My header would look like:

// sw.h

typedef struct tag_watch_state sw;

sw *sw_create();
void sw_start(sw *);
void sw_split(sw *);
void sw_stop(sw *);
void sw_destroy(sw *);


However, if you have a big program, with a lot of states, each requiring a non-trivial amount of code, it would make sense to have one .c file for each state. Even then though, I would endeavour to present only one abstract header to the external callers.

Code Snippets

struct watch_state {
    event_start_func_f start;
    event_split_func_f split;
    event_stop_func_f stop;

    const char *name;
};

struct watch_state started  = {default_start, split, stop, "Started"};
struct watch_state stopped  = {start, split, default_stop, "Stopped"};
struct watch_state splitted = {default_start, split, stop, "Splitted"};
struct tag_watch_state {
    /* State */
    struct watch_state *state;

    int time;
};
static void start(watch_state_t *state)
{
    unsigned int secs = 5;
    unsigned int i = 0;

    /* Do some work here to simulate stop*/
    for(i = 0; i < secs; i++) {    
        sleep(1);
        printf("Starting the stopwatch ...%u\n", i);
    }

    /* Update state */
    state->state = &started;
    state->time = 0;

    printf("[ %s ] state [ %s ] time [ %d ]\n", __func__, state->name, state->time);
}
// sw.h

typedef struct tag_watch_state sw;

sw *sw_create();
void sw_start(sw *);
void sw_split(sw *);
void sw_stop(sw *);
void sw_destroy(sw *);

Context

StackExchange Code Review Q#35754, answer score: 9

Revisions (0)

No revisions yet.