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

Static variable representing program state accessed with multiple functions

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

Problem

I have this method that performs certain functions based on the state that the algorithm is currently at:

static infoStruct_t info;

void first_part_of_algorithm(void){
    info.state = FIRST_STATE;
    //set other information here as well
    while(info.state < FINAL_STATE){
        switch(info.state){

            case FIRST_STATE:
                perform_first_state_function();
                break;

            case SECOND_STATE:
                perform_second_state_function();
                break;

            case THIRD_STATE:
                perform_check_needed_for_algorithm();
                break;

            default:
                //should never be reached
                break;

        }
    }

}


Now in each of these "perform" functions, the state can change. The first function always causes the state to go to the second state, the second function always makes the state go to the third state, and the third function will either reset the state to the first state, or go to the final state and leave the first part of the algorithm. Also, different parts of the info struct are set at various points in any of the functions.

I have not yet written the second part of the algorithm, but it will need to have access to the info struct

Here are the following state / check functions:

```
void perform_first_state_function(void){
while(wait_for_condition()!=SUCCESS){
sleep();
}
info.state = SECOND_STATE;
sleep();
}

void perform_second_state_function(void){
int attempts = 0;
while(attempts < ALLOWED_ATTEMPTS){
if(event_is_ready()){
if(info.detection_number==0){
//set various info properties
...
info.first_detection_time = get_time();
info.detection_number = 1;
break;
}
else{
//set other info properties
...
info.second_detection_time = get_

Solution

Since it's a single-threaded algorithm, I think you could simplify things a bit to avoid all the state-checking. Unless you're reading the state from another thread, you don't really need to keep track of it at all. You could simply make perform_check_needed_for_algorithm return a bool:

for (;;)
{
    /* info.state = FIRST_STATE; */
    perform_first_state_function();
    /* info.state = SECOND_STATE; */
    perform_second_state_function();
    /* info.state = THIRD_STATE; */
    if (perform_check_needed_for_algorithm())
    {
        /* info.state = FINAL_STATE; */
        break;
    }
}


The commented-out state assignments are just there to show what's happening at each stage. Although a state machine's inner functions ought to be responsible for updating the state, in an example as simple as this, it's arguably more readable to update the state in the outer function.

If you need access to the state from another thread, then you'll need to update it using atomic operations, and mark it as volatile.

Afterthought:
perform_second_state_function doesn't always advance the state. It fails once ALLOWED_ATTEMPTS is exceeded. However, it dumps you back into the outer loop, and dives straight back into perform_second_state_function because state.info is still SECOND_STATE. Bit of a design error there IMO - it will keep trying no matter what.

Code Snippets

for (;;)
{
    /* info.state = FIRST_STATE; */
    perform_first_state_function();
    /* info.state = SECOND_STATE; */
    perform_second_state_function();
    /* info.state = THIRD_STATE; */
    if (perform_check_needed_for_algorithm())
    {
        /* info.state = FINAL_STATE; */
        break;
    }
}

Context

StackExchange Code Review Q#56652, answer score: 4

Revisions (0)

No revisions yet.