0

I'm having different different types of structs, which are going to be passed to a function which performs the same tasks on them.

int menu_parameter_arrow_print(game_setting_identifier* identifier, controller_direction direction, uint8_t position)
{
    if((position > setting->alternatives_number) || position < 0)
    {
        #ifdef OLED_PRINT_DEBUG_ENABLE
        OLED_debug_print("Out of bounds");
        #endif
        return RETURN_VALUE_FAILURE;
    }
    else
    {

        switch ((int)*identifier)
        {
            case ((int) GAME_SETTING_ANALOG):
            game_setting_analog* setting = (game_setting_analog*)&identifier;
            case ((int) GAME_SETTING_TOGGLE):
            game_setting_toggle* setting = (game_setting_toggle*)&identifier;
            case ((int) GAME_SETTING_VALUE):
            game_setting_value* setting = (game_setting_value*)&identifier;
        }

This function gives a conflicting type-error

The operations performed on the structs are the same, but the structs contains different types of members:

struct game_setting_analog
{
    //Identifier for the game-setting type:
    game_setting_identifier identifier;
    //Alternatives:
    char* alternatives[4];
};
typedef struct game_setting_value game_setting_value;

struct game_setting_value
{
    game_setting_identifier identifier;
    uint8_t* alternatives[6];
    uint8_t alternatives_number;    
};
typedef struct game_setting_toggle game_setting_toggle;

struct game_setting_toggle
{
    //Identifier for the game-setting type:
    game_setting_identifier identifier;
    toggle_state* alternatives[2];
};
typedef struct game_setting_difficulty game_setting_difficulty;

struct game_setting_difficulty
{
    game_setting_identifier identifier;
    char* alternatives[3];
};

Actions will be performed on the 'alternatives'-member of the structs, even though these members are of different types.

Is there a solution to doing this without having to use one if-statement for each identifier?

Edit: With a modification to the switch-case, I'm able to get the initialization compiled. The variables inside the switch-scope is however not visible to the rest of the function

int menu_print_parameter_line(game_setting_identifier* identifier, controller* C, uint8_t position)
{
    uint8_t next_position = position;
    controller_direction  previous_direction = C->joystick.generalDirection;

    if ((identifier == NULL) || (C == NULL) || (position == NULL))
    {
        return -1;
    }

    switch((int) identifier)
    {
        case ((int) GAME_SETTING_ANALOG):
        {
            game_setting_analog* setting = (game_setting_analog*)identifier;
            uint8_t alternatives_number = 4;
        }
        break;
        case ((int) GAME_SETTING_TOGGLE):
        {
            game_setting_toggle* setting = (game_setting_toggle*)identifier;
            uint8_t alternatives_number = 2;
        }
        break;
        case ((int) GAME_SETTING_VALUE):
        {
            game_setting_value* setting = (game_setting_value*)identifier;
            uint8_t alternatives_number = setting->alternatives_number;
        }
        break;
        default:
        {
            return -1;
        }
        break;
    }

    #ifdef MENU_PARAMETER_ASSIGNMENT_DEBUG
        OLED_debug_print("before switch-case");
    #endif
    switch (previous_direction)
    {
        case LEFT:
        next_position -= 1;
        if(next_position <= 0)
        {
            next_position = alternatives_number;
        }

Jonas Hjulstad
  • 395
  • 1
  • 8

2 Answers2

1

I personally don't like the inheritance model that depends on the first member of the structure, like the BSD socket library is using. Basically you are just trying to implement std::variant from c++ in C.

Is there a solution to doing this without having to use one if-statement for each identifier?

The object-oriented concept of interface works very nice and I believe is applicable in this case. It takes some C discipline to write it, but it works like a charm and you could be looking for it here.

I copied your definitions from which I removed typedefs because I don't like them:

struct game_setting_analog {
    char* alternatives[4];
};

struct game_setting_value {
    uint8_t* alternatives[6];
    uint8_t alternatives_number;    
};

struct game_setting_toggle {
    toggle_state* alternatives[2];
};

struct game_setting_difficulty {
    char* alternatives[3];
};

Let's first implement the interface abstraction with a function pointer that allows to get the alternatives number:

// forward definition
struct game_setting_s;

// the virtual table for game_settings
struct game_setting_vtable_s {
    uint8_t (*get_alternatives_number)(struct game_setting_s *t);
    // TODO: add other members, constructor, copy constructor, destructor, etc.
};

// represents any game_setting
// exposes a public interface to access and manipulate a game_setting
struct game_setting_s { 
   // the vtable is const, so it can save RAM
   const struct game_setting_vtable_s *v;
   // this is a pointer to private settings data
   void *data;
};

// accessor for less (or more ;) typing
static inline
uint8_t game_setting_get_alternatives_number(struct game_setting_s *t) {
      // alternative you could pass t->data to the function, I pass it all
      // so that functions can modify the t->data member
      // and also so that advanced functions usages can use like container_of macros
      return t->v.get_alternatives_number(t);
}    

Then you need to provide the virtual tables for each of the types. The definitions can be in separate types, so you can have a separate .c/.h file pair for each of the type, just exposing public interface.

// game_setting_analog --------------------

static
uint8_t game_setting_analog_get_altenatives_number(struct game_setting_s *t) 
{
     return 4;
}

const struct game_setting_vtable_s game_setting_analog_vtable = {
      .get_alternatives_number = game_setting_analog_get_altenatives_number,
};

// game_setting_toggle --------------------

static
uint8_t game_setting_toggle_get_altenatives_number(struct game_setting_s *t) {
     struct game_setting_toggle *data = t->data;
     return data->alternatives_number;
}

const struct game_toggle_vtable_s game_setting_toggle_vtable = {
      .get_alternatives_number = game_setting_toggle_get_altenatives_number,
};

// and so on...

Then your function takes just the interface and is very clear without any switch case:

int some_function_that_needs_to_know_which_setting_is_passed(struct game_setting_s *s) {
    int number_of_alternatives = game_setting_get_alternatives_number(s);
}

Remember to construct the interface object properly and watch who owns the memory of the object. Let's construct a toggle and call out function:

struct game_settting_toggle memory;

// your function to initialize the toggle
game_setting_toggle_intialize(&memory);

// the interface is constructed with the proper vtable
// and a pointer to proper memory region with the data
struct game_setting_s any_setting = {
     .vtable = game_setting_toggle_vtable,
     .data = &memory,
};

// the initailize function could be in interface too
// so you would just call game_setting_initialize(&any_setting);
// with usage of dynamic allocation, you can just ex. 
// struct game_setting_s *any_setting = game_setting_new_toggle();
// and write proper object-oriented factories

// finally call our function.
some_function_that_needs_to_know_which_setting_is_passed(&any_setting);
KamilCuk
  • 120,984
  • 8
  • 59
  • 111
0

Case labels do not provide scopes for variables. All three setting variables within the switch have different types which are the conflicts the compiler. Use brackets to define scopes:

switch ((int)*identifier)
  {
        case ((int) GAME_SETTING_ANALOG):
        {
            game_setting_analog* setting = (game_setting_analog*)&identifier;
        }
        case ((int) GAME_SETTING_TOGGLE):
        {
            game_setting_toggle* setting = (game_setting_toggle*)&identifier;
        }
        case ((int) GAME_SETTING_VALUE):
        {
            game_setting_value* setting = (game_setting_value*)&identifier;
        }
    }

Also, you're not breaking in the cases, so the code in all three cases are run if ((int)*identifier == (int) GAME_SETTING_ANALOG)

ikkentim
  • 1,639
  • 15
  • 30