Skip to main content
Commonmark migration
Source Link

Thanks for sharing your code.

This looks pretty good for a novices code since you managed to remove some code duplication by using parameterized methods.

here is what I would change:

Naming

#Naming FindingFinding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

###Naming Conventions

Naming Conventions

Please read (and follow) the Java Naming Conventions.

eg .: variables of type boolean (like gameOver) or methods returning a boolean should start with *is, has, can or alike.

Don't surprise your readers

###Don't surprise your readers YourYour method askForTurn() does nor only acquire user input, it actually does the turn. So it should better be named doTurn().

Magic numbers

#Magic numbers replacereplace the literal numbers by constants with expressive names.

Separation of Concerns

#Separation of Concerns askForTurn() does more than one thing:

  • it acquires user input,
  • validates the user input
  • translates the input into a field position
  • checks the end condition

each of this actions should live in a method of its own.

hidden recursion/implicit error handling

#hidden recursion/implicit error handling InIn case of an invalid input you do a recursive call to askForTurn(). Beside that this also belongs in the Don't surprise your readers category it is a potential memory leak.

Use of the static key word

#Use of the static key word TheThe only method that should be static in your code should be main. For the rest of your code you should avoid the static key word unless you have a good reason to use it.

In your case it would be quiet easy to do:
move the while loop from main into a new (non static) method and call it on an instance of the class:

public static void main(String[] args) {
    new TicTacToe().runTheGame();
}

This would safe you from all the statics in your program.

Thanks for sharing your code.

This looks pretty good for a novices code since you managed to remove some code duplication by using parameterized methods.

here is what I would change:

#Naming Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

###Naming Conventions

Please read (and follow) the Java Naming Conventions.

eg .: variables of type boolean (like gameOver) or methods returning a boolean should start with *is, has, can or alike.

###Don't surprise your readers Your method askForTurn() does nor only acquire user input, it actually does the turn. So it should better be named doTurn().

#Magic numbers replace the literal numbers by constants with expressive names.

#Separation of Concerns askForTurn() does more than one thing:

  • it acquires user input,
  • validates the user input
  • translates the input into a field position
  • checks the end condition

each of this actions should live in a method of its own.

#hidden recursion/implicit error handling In case of an invalid input you do a recursive call to askForTurn(). Beside that this also belongs in the Don't surprise your readers category it is a potential memory leak.

#Use of the static key word The only method that should be static in your code should be main. For the rest of your code you should avoid the static key word unless you have a good reason to use it.

In your case it would be quiet easy to do:
move the while loop from main into a new (non static) method and call it on an instance of the class:

public static void main(String[] args) {
    new TicTacToe().runTheGame();
}

This would safe you from all the statics in your program.

Thanks for sharing your code.

This looks pretty good for a novices code since you managed to remove some code duplication by using parameterized methods.

here is what I would change:

Naming

Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

Naming Conventions

Please read (and follow) the Java Naming Conventions.

eg .: variables of type boolean (like gameOver) or methods returning a boolean should start with *is, has, can or alike.

Don't surprise your readers

Your method askForTurn() does nor only acquire user input, it actually does the turn. So it should better be named doTurn().

Magic numbers

replace the literal numbers by constants with expressive names.

Separation of Concerns

askForTurn() does more than one thing:

  • it acquires user input,
  • validates the user input
  • translates the input into a field position
  • checks the end condition

each of this actions should live in a method of its own.

hidden recursion/implicit error handling

In case of an invalid input you do a recursive call to askForTurn(). Beside that this also belongs in the Don't surprise your readers category it is a potential memory leak.

Use of the static key word

The only method that should be static in your code should be main. For the rest of your code you should avoid the static key word unless you have a good reason to use it.

In your case it would be quiet easy to do:
move the while loop from main into a new (non static) method and call it on an instance of the class:

public static void main(String[] args) {
    new TicTacToe().runTheGame();
}

This would safe you from all the statics in your program.

Source Link

Thanks for sharing your code.

This looks pretty good for a novices code since you managed to remove some code duplication by using parameterized methods.

here is what I would change:

#Naming Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

###Naming Conventions

Please read (and follow) the Java Naming Conventions.

eg .: variables of type boolean (like gameOver) or methods returning a boolean should start with *is, has, can or alike.

###Don't surprise your readers Your method askForTurn() does nor only acquire user input, it actually does the turn. So it should better be named doTurn().

#Magic numbers replace the literal numbers by constants with expressive names.

#Separation of Concerns askForTurn() does more than one thing:

  • it acquires user input,
  • validates the user input
  • translates the input into a field position
  • checks the end condition

each of this actions should live in a method of its own.

#hidden recursion/implicit error handling In case of an invalid input you do a recursive call to askForTurn(). Beside that this also belongs in the Don't surprise your readers category it is a potential memory leak.

#Use of the static key word The only method that should be static in your code should be main. For the rest of your code you should avoid the static key word unless you have a good reason to use it.

In your case it would be quiet easy to do:
move the while loop from main into a new (non static) method and call it on an instance of the class:

public static void main(String[] args) {
    new TicTacToe().runTheGame();
}

This would safe you from all the statics in your program.