cloopsoptimizationstructstatic

How to create a new instance of a given type (static) in every iteration of a loop


I am currently trying to learn how to code in C, so I decided to create a project to keep track of bets during a Poker Game. I declared my structure Player as it follows :

typedef struct Player Player;
struct Player
{
    char Role[11];
    char Name[20];
    char State[7]; 
    int Money;
    int Bet;
};

I only use static tables on purpose for optimization purposes.

void Register_Player_Finalisation(char *Name, int Money, char *Role_g, char *State_g, Player *Player_created)
{
    /**/
    strcpy(Player_created->Name, Name);

    strcpy(Player_created->Role, Role_g);

    strcpy(Player_created->State, State_g);

    Player_created->Money = Money;
    Player_created->Bet = 0;
}

I make use of Side Effects because it seems more 'clean' to do it that way

Here comes my issue :

int Register_Player(Player *Record_Of_Payers)
{
    int Players_active = 1;
    char Name[20];
    scanf("%s\n", Name);
    char Role[20] = "Not_Yet";
    char State[8] ="Not_Yet";
    int Money = 2000;
    Player Player_created;
    Register_PLayer_Finalisation(Name, Money, Role,State, &Player_created);
    Record[0] = Player_created;

    /* Boucle pour ajouter des joueurs */
    while (Players_active < 6 )
    {
        
        scanf("%s\n", Name);
        if (*Name != '7')
        {
            Players_active++;
            /* Here is the issue */
            Player New_Player;
            /*------------------*/
            Register_Player_Finalisation(Name, Money, Role,State, &New_Player);
            Record[Players_active - 1] = New_Player;
        }
        else
        {
            printf("Exception \n");
            break;
        }
        
    }
    return Players_active;

}

So I'd like to create a new instance of Player each time we loop. I could do a malloc and then free it, but I'd like to avoid to use dynamics tables. I also have a little issue, when we give 7 as an argument it does not stop right away but needs another character (not matter which one) to break.

It is working as of now, but I feel like it should not.


Solution

  • How to create a new instance of a given type (static) in every iteration of a loop

    As you write "static" the answer is: You can't.

    Static variables (aka objects with static storage duration) are created when a program starts and they exists until the program ends. There are no way to add additional static variables at run time.

    From your question it's clear that you are aware that a "common solution" is to use dynamic allocation (i.e. malloc and friends).

    But you also say you don't want that...

    In that case you need to "create" the array "early" in program execution so that it will be available for all functions.

    For instance:

    #define MAX_PLAYERS 16
    
    void do_stuff(Player* players, size_t max_players);
    
    int main(void) {
        static Player players[MAX_PLAYERS];
        ...
        do_stuff(players, MAX_PLAYERS);
        ...
        return 0;
    }
    

    Obviously this will only work when you can define a limit for the maximum number of players supported by your program.

    If you don't want such a "hard coded limit"... well, then you have to accept doing dynamic allocation.

    Comments for your code

    The function Register_Player has the parameter Record_Of_Payers but inside the function it's never used. Instead the function uses Record. Probably just a typo-like bug (did you edit something while posting the question?).

    BTW: Record_Of_Payers seems a typo itself - probable an l is missing.

    Another typo-like thing is that the Player struct has char State[7]; but other part of the code has char State[8] ="Not_Yet";. You probably wanted the same array size (i.e. 8) both places.

    Then you use scanf :-(

    Don't !!!

    Forget all about scanf. Never use it again.

    Take input using fgets and parse the input afterwards.

    (BTW: your current use of scanf allows users to overflow your buffers so that all kind of ugly things may happen. No need to tell how to fix that as you will switch to fgets going forward.)

    I only use static tables on purpose for optimization purposes.

    Forget optimization!

    In general it's seldom that you need to care. For your specific program, it's absolutely not something to care about. Your program takes input from the user. Doing that means that the performance of all other code won't matter.

    Sure - sometimes performance matters. But in most case you should start by writing clean, simple code. If it turns out not to meet the performance requirements, you profile the program and then optimize the bottlenecks. Don't do "funny/useless/unneed" tricks from the start.

    I make use of Side Effects because it seems more 'clean' to do it that way

    It's not clear to me exactly what you mean by this. But if 'clean' means something like "good practice", "maintable code" (or similar) I would say that there are room for improvements.

    A few examples:

    int Register_Player(Player *Record_Of_Payers)
    

    The name of the function suggest that it will add a single Player but actually it adds multiple Players.

    When passing an array (or rather a pointer to the first element of an array) it's (nearly) always a good idea to pass the array size. Especially when it's a function that are supposed to put data into the array. When passing a size, the function can make sure not to write out of bounds.

    int Register_Players(Player *Record_Of_Players, size_t Max_Players)
    

    and then do

    while (Players_active < Max_Players)
    

    instead of using a hard code value (i.e. 6) like you do now.

    Your use of Players_active is also "strange" and doesn't match the "common" way of doing such things.

    You increment Players_active before adding the new element to the array. Consequently, you have to subtract 1 when adding the new element. It's not a bug but it makes the code look strange.

    The "common" way is more like:

            Record_Of_Players[Players_active] = New_Player;
            Players_active++;
    

    or the shorter form:

            Record_Of_Players[Players_active++] = New_Player;
    

    I would also initialize Players_active to zero instead of one and then increment after adding the first player.