#Keep at it!
#Keep at it!
Keep at it!
You realize that this is actually (char* board) because arrays are not passed to functions. Generally, don’t pass a pointer to a sequence like this, whether length is passed separately or implicitly known. See ⧺I.13.
/// still typing … ///
Function winnerCheck is another example of repeated code. You can see that the win/lose messages and return values are stated three times, with a pair of nearly identical checks in each repetition.
Instead, call a separate function that just returns a code indicating the winner (or lack of) and then print the message based on the result. And only have one copy of the print code, using the variable to state which player it was.
The actual checking can be done more algorithmically, not expanded out into individual tests like that.
I see you did manage to not repeat the checking for X and O, but test for a row/col/diag of the same mark. That is better than most of the other code here.
The main is two copies of the same block inside the loop. After making a single player(param) function rather than player1 and player2, can you see how to make one loop that alternates players on each iteration?
#Keep at it!
Please post your next version after making a Player class and not repeating code, and I’ll be happy to look at it again.
You realize that this is actually (char* board) because arrays are not passed to functions. Generally, don’t pass a pointer to a sequence like this, whether length is passed separately or implicitly known.
/// still typing … ///
You realize that this is actually (char* board) because arrays are not passed to functions. Generally, don’t pass a pointer to a sequence like this, whether length is passed separately or implicitly known. See ⧺I.13.
Function winnerCheck is another example of repeated code. You can see that the win/lose messages and return values are stated three times, with a pair of nearly identical checks in each repetition.
Instead, call a separate function that just returns a code indicating the winner (or lack of) and then print the message based on the result. And only have one copy of the print code, using the variable to state which player it was.
The actual checking can be done more algorithmically, not expanded out into individual tests like that.
I see you did manage to not repeat the checking for X and O, but test for a row/col/diag of the same mark. That is better than most of the other code here.
The main is two copies of the same block inside the loop. After making a single player(param) function rather than player1 and player2, can you see how to make one loop that alternates players on each iteration?
#Keep at it!
Please post your next version after making a Player class and not repeating code, and I’ll be happy to look at it again.
Don’t write using namespace std;.
Read through and bookmark the C++ Standard Guidelines.
Numbers I note later are citations from this.
#define SIZE 9 Don't use #define for constants or “functions” (⧺ES.31)
I see this is only used in one place, in main, so why do you even need a name for it? You can have the array automatically determine the size based on the initializer:
char board[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8'}; void printCurrentBoard(char board[]) You realize that this is actually (char* board) because arrays are not passed to functions. Generally, don’t pass a pointer to a sequence like this, whether length is passed separately or implicitly known.
You should make the board an object. Then this can be a member function that doesn’t need a parameter.
You should have made it const char* since the function does not modify the data.
The body is very repetitive! Use a loop and don’t repeat code en masse.
player1 and player2 have the same parameter issue.
The functions are identical except for one data item!! Don’t Do That! Factor out the thing that changed and make it a parameter.
Since “player 1” and “player 2” are not very descriptive anyway, I’ll use the X and O for the prompt, too.
void player (const char Mark, char* board) { int move; cout << '\n' << Mark << " player's turn: "; cin >> move; while(1) if (!free_space(board[move])) { cout << "Invalid move, try again: "; cin >> move; } else { break; } board[move] = Mark; } Your test allowed a player to skip a turn by playing on a square he already had. It doesn’t matter which player is moving; only unoccupied squares are available. So I broke the test out into another function so as not to clutter the code. It’s readable with the name free_space, without worrying too much about how it actually makes that determination, right?
Your loop is structured awkwardly. Use a post test loop:
do { cout << "Invalid move, try again: "; cin >> move; } while (!free_space(board[move])); Note that your input function still doesn’t verify that a valid number was entered. Type 22 for example and it will trash the stack — very bad!
Make getting a legal input from 0 to 8 a separate function that is concerned only with that, and doesn’t return until it gets a good value.
/// still typing … ///
lang-cpp