2
\$\begingroup\$

I have a working Tic-Tac-Toe game, which requires two players. The game also keeps track of the result of each game and assigns score to each player based on the result of the game.

For this, I have made use of structures:

Player Structure

typedef struct
{   
    int score[10];
    char choice;
    char name[10];
    bool turn;

}Player;

Game Structure

I have made use of another structure for the game setup, which contains the game board, the valid positions available, and also the number of games played:

typedef struct
{
    char board[3][3];
    bool validPos[10];
    int games;

}Game;

main function

 // included only a part of the main function that I am concerned about;
int main(void)
{
    Game *game = malloc(sizeof(Game));
    Player *player_1 = malloc(sizeof(Player));
    Player *player_2 = malloc(sizeof(Player));
}

Functions used in the game:

void assignXO(Player *, Player *);
void startGame(Game *, Player *, Player *);
void display(Game *, Player *, Player *);
bool victoryCheck(Game *);
bool isComplete(Game *);
void playerMove(Game *, Player *);
void declareWinner(Game *, Player *, Player *);
void gameReset(Game *);
void scoreBoard(Player *, Player *, int);

Queries

  1. As you can observe the number of arguments passed to the functions are dependent on the purpose of the function. Is it wise to declare global variables for the structure so that potentially no arguments are needed to be passed to any of the functions?

  2. Is it good practice to have more than 2 arguments passed to the functions? Does it help with conveying the logic to the reader?

  3. If I used the structure Player within the Game structure, will it make the code any better in terms of readability and how well the code is organised?

\$\endgroup\$
1
\$\begingroup\$
  1. Well, it's a good idea to minimize dependency on global state.
    Doing so enhances testability and reduces dependencies, especially the hidden type.

    Still, if you have such pervasively used state as Game, it's a valid thought to make it global instead of passing it around everywhere.

  2. Pass the arguments needed, no more, no less.

  3. It could be. Or not. In any case, it would reduce your flexibility in using your code for example for a tic-tac-toe contest.

Now lets critic your code:

  1. validPos seems to be redundant, being a simple function of board. Or is there some extra magic?

  2. Avoid dynamic allocation where it doesn't help. That means the memory-used is scoped and the block is reasonably small, like those allocations in main.

  3. Anyway, avoid repeating your types to avoid mismatches. It should have been:

    Game *game = malloc(sizeof *game);
    
  4. Parameter names are valuable space for documenting an arguments use.
    At least scoreBoard and declareWinner could use a bit more:

    void declareWinner(Game *, Player* winner, Player* looser);
    void scoreBoard(Player *, Player *, int round);
    // documented my guesses what the pertinent arguments mean
    
\$\endgroup\$
  • \$\begingroup\$ could you elaborate on point 3, on repeating types ? \$\endgroup\$ – CaptainDaVinci Feb 8 '17 at 8:34
  • 2
    \$\begingroup\$ Well, see the code I put after that, it shows how to lock malloc and the variable together properly so you don't have to repeat the type. Admittedly, as the variable is introduced on that line it's a tame case, but the same holds when they are separated. \$\endgroup\$ – Deduplicator Feb 8 '17 at 9:13
1
\$\begingroup\$

I like your current design. If you are going to have one game at a time, globals reduce the number of parameters. However, what if you expand to have multiple games going at once? More then two players?

Dividing it so that there's Player and Game is good practice, kind of like normalising a database https://en.wikipedia.org/wiki/Boyce%E2%80%93Codd_normal_form. In the spirit of BCNF, I think that Game should definitely have two players. This will reduce the number of arguments that your functions take and is good for https://en.wikipedia.org/wiki/Single_source_of_truth; for example, what happens when you void playerMove(Game *, Player *) a person that is out-of-order or not even associated with the game?

struct Player {
    int score[10];
    int firstScore, lastScore;
    char name[128];
};

struct Game {
    int noGames;
    bool isFinished;
    char board[3][3];
    struct Player *x, *o, *winner; /* winner is null until winner */
    enum { TURN_X, TURN_O } turn;
};

(You could get cute and enum Turn { TURN_X, TURN_O, TURN_NO } and in the struct, struct Players players[TURN_NO]; enum Turn turn; so that you could refer to game->players[game->turn]) That way, you simplify your functions and split them into two classes, Player and Game. Do things that have to be done in a certain order at once; for example, calling startGame should assign the players and call gameReset; assignXO should be done there.

struct Player *newPlayer(char *);
void displayPlayer(struct Player *);

struct Game *startGame(struct Player *, struct Player *);
void display(struct Game *);
bool scoreBoard(struct Game *); /* returns isFinished */
void playerMove(struct Game *);
int gameReset(struct Game *); /* returns noGames */
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.