Skip to main content
Code Review

Return to Answer

added 137 characters in body
Source Link
TorbenPutkonen
TorbenPutkonen
  • 7.9k
  • 20
  • 28

First of all, look up SOLID pronciples and study them. Especially the "S" which stands for "single responsibility principle": each component should be responsible for one thing only. These are the most important things I have learned as a programmer and I wish I had learned them 10 years sooner.

Board

By the name I would assume Board class contains the state of the game: the size of the board and the locations of played moves. Nothing else. I might think that it could even be a generic board class that could be used for chess, checkers, etc. The class however is responsible for managing user input, player turn, etc. There's output written in the middle of the game logic controlled by boolean flags. A lot of responsibilities violating the "S" in SOLID. It's a bit of spaghetti code.

TicTacToe

The TicTacToe class has very little to do with "Tic Tac Toe" the game. It is just a generic "main loop" for a turn based game. The fundamentals of "Tic Tac Toe" are actually written into the Board class. Whatever is in the TicTacToe could as well be put into the Main class. The naming is thus a bit misleading.

I would refactor the classes so that Board only contains the game state without other logic. It would provide a method for checking what a board location contains (X, O or empty) and a method for placing a marker in a location. TicTacToe then provides metodsa metod for making movesa move while enforcing legality and information of whose turn it is and whether the game has ended or not. It would thus contain the rules to the Tic Tac Toe game.

Then I would write a new class that would only handle reading user input and printing the game state to console. Maybe it could be called "TicTacToeCLI" (for command line interface). And the main class would set these three classes up and run the main loop.

First of all, look up SOLID pronciples and study them. Especially the "S" which stands for "single responsibility principle": each component should be responsible for one thing only. These are the most important things I have learned as a programmer and I wish I had learned them 10 years sooner.

Board

By the name I would assume Board class contains the state of the game: the size of the board and the locations of played moves. Nothing else. I might think that it could even be a generic board class that could be used for chess, checkers, etc. The class however is responsible for managing user input, player turn, etc. There's output written in the middle of the game logic controlled by boolean flags. A lot of responsibilities violating the "S" in SOLID. It's a bit of spaghetti code.

TicTacToe

The TicTacToe class has very little to do with "Tic Tac Toe" the game. It is just a generic "main loop" for a turn based game. The fundamentals of "Tic Tac Toe" are actually written into the Board class. Whatever is in the TicTacToe could as well be put into the Main class. The naming is thus a bit misleading.

I would refactor the classes so that Board only contains the game state without other logic. TicTacToe then provides metods for making moves while enforcing legality and information of whose turn it is and whether the game has ended or not. It would thus contain the rules to the Tic Tac Toe game.

Then I would write a new class that would only handle reading user input and printing the game state to console. Maybe it could be called "TicTacToeCLI" (for command line interface). And the main class would set these three classes up and run the main loop.

First of all, look up SOLID pronciples and study them. Especially the "S" which stands for "single responsibility principle": each component should be responsible for one thing only. These are the most important things I have learned as a programmer and I wish I had learned them 10 years sooner.

Board

By the name I would assume Board class contains the state of the game: the size of the board and the locations of played moves. Nothing else. I might think that it could even be a generic board class that could be used for chess, checkers, etc. The class however is responsible for managing user input, player turn, etc. There's output written in the middle of the game logic controlled by boolean flags. A lot of responsibilities violating the "S" in SOLID. It's a bit of spaghetti code.

TicTacToe

The TicTacToe class has very little to do with "Tic Tac Toe" the game. It is just a generic "main loop" for a turn based game. The fundamentals of "Tic Tac Toe" are actually written into the Board class. Whatever is in the TicTacToe could as well be put into the Main class. The naming is thus a bit misleading.

I would refactor the classes so that Board only contains the game state without other logic. It would provide a method for checking what a board location contains (X, O or empty) and a method for placing a marker in a location. TicTacToe then provides a metod for making a move while enforcing legality and information of whose turn it is and whether the game has ended or not. It would thus contain the rules to the Tic Tac Toe game.

Then I would write a new class that would only handle reading user input and printing the game state to console. Maybe it could be called "TicTacToeCLI" (for command line interface). And the main class would set these three classes up and run the main loop.

Source Link
TorbenPutkonen
TorbenPutkonen
  • 7.9k
  • 20
  • 28

First of all, look up SOLID pronciples and study them. Especially the "S" which stands for "single responsibility principle": each component should be responsible for one thing only. These are the most important things I have learned as a programmer and I wish I had learned them 10 years sooner.

Board

By the name I would assume Board class contains the state of the game: the size of the board and the locations of played moves. Nothing else. I might think that it could even be a generic board class that could be used for chess, checkers, etc. The class however is responsible for managing user input, player turn, etc. There's output written in the middle of the game logic controlled by boolean flags. A lot of responsibilities violating the "S" in SOLID. It's a bit of spaghetti code.

TicTacToe

The TicTacToe class has very little to do with "Tic Tac Toe" the game. It is just a generic "main loop" for a turn based game. The fundamentals of "Tic Tac Toe" are actually written into the Board class. Whatever is in the TicTacToe could as well be put into the Main class. The naming is thus a bit misleading.

I would refactor the classes so that Board only contains the game state without other logic. TicTacToe then provides metods for making moves while enforcing legality and information of whose turn it is and whether the game has ended or not. It would thus contain the rules to the Tic Tac Toe game.

Then I would write a new class that would only handle reading user input and printing the game state to console. Maybe it could be called "TicTacToeCLI" (for command line interface). And the main class would set these three classes up and run the main loop.