HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Basic Noughts & Crosses

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
noughtsbasiccrosses

Problem

So I got bored and made a simple noughts and crosses game with colours instead of symbols.

The colours are red and blue and I made the GUI in NetBeans with my own code in combination with the NetBeans GUI Editor.

My Code:

```
package com.coal;

import java.util.Random;

public class Noughts extends javax.swing.JFrame {

public Noughts() {
if (ran == 0) {
move = "Red";
} else {
move = "Blue";
}
initComponents();

}

private void checkForVictory() {
//Horizontal Victories
if (color1 == 1 && color2 == 1 && color3 == 1) {
winner = "Red";
System.out.println("Winner!" + winner);
gameOver = true;
jTextPane1.setText("Winner is " + winner);
}
if (color1 == 2 && color2 == 2 && color3 == 2) {
winner = "Blue";
System.out.println("Winner!" + winner);
gameOver = true;
jTextPane1.setText("Winner is " + winner);
}

if (color4 == 1 && color5 == 1 && color6 == 1) {
winner = "Red";
System.out.println("Winner!" + winner);
gameOver = true;
jTextPane1.setText("Winner is " + winner);
}
if (color4 == 2 && color5 == 2 && color6 == 2) {
winner = "Blue";
System.out.println("Winner!" + winner);
gameOver = true;
jTextPane1.setText("Winner is " + winner);
}

if (color7 == 1 && color8 == 1 && color9 == 1) {
winner = "Red";
System.out.println("Winner!" + winner);
gameOver = true;
jTextPane1.setText("Winner is " + winner);
}
if (color7 == 2 && color8 == 2 && color9 == 2) {
winner = "Blue";
System.out.println("Winner!" + winner);
gameOver = true;
jTextPane1.setText("Winner is " + winner);
}

//Vertical Victories
if (color1 == 1 && color4 == 1 && color7 == 1) {
winner = "Red";
System.out.println("Winner!" + winner);
gameOver = true;
jTextPane1.setText("Winner is " + winner);
}
i

Solution

remove code duplication

You have lots of duplicated code all over the program which could be reduced by the use of parameterized methods. E.g. the checkForVictory() method could be changed like this:

private void ckeckLine(int firstField, int middleField, int lastField, int playerColor, String playerName){
  if(firstField==playerColor&&middleField==playerColor&&lastField==playerColor){
       winner = playerName;
       System.out.println("Winner!" + winner);
       gameOver = true;
       jTextPane1.setText("Winner is " + winner);
  }
}

private void checkForVictoryForPlayer(int playerColor, String playerName ) {
    //Horizontal Victories
     ckeckLine(color1, color2, color3, playerColor ,playerName);
     ckeckLine(color4, color5, color6, playerColor ,playerName);
    // you get the idea?
}

private void checkForVictory(){
   checkForVictoryForPlayer(1, "Red");
   checkForVictoryForPlayer(2, "Blue");
   //Draw Check
   if(color1 != 0 && color2 != 0 && color3 != 0&& color4 != 0&& color5 != 0&& color6 != 0&& color7 != 0&& color8 != 0&& color9 != 0 && winner == ""){
      System.out.println("Draw!");
      gameOver = true;
       jTextPane1.setText("Draw!");
  }
}


Same is true for your b?actionPerformed() methods. There could be only one parameterized version instead of 9.

Code Snippets

private void ckeckLine(int firstField, int middleField, int lastField, int playerColor, String playerName){
  if(firstField==playerColor&&middleField==playerColor&&lastField==playerColor){
       winner = playerName;
       System.out.println("Winner!" + winner);
       gameOver = true;
       jTextPane1.setText("Winner is " + winner);
  }
}


private void checkForVictoryForPlayer(int playerColor, String playerName ) {
    //Horizontal Victories
     ckeckLine(color1, color2, color3, playerColor ,playerName);
     ckeckLine(color4, color5, color6, playerColor ,playerName);
    // you get the idea?
}


private void checkForVictory(){
   checkForVictoryForPlayer(1, "Red");
   checkForVictoryForPlayer(2, "Blue");
   //Draw Check
   if(color1 != 0 && color2 != 0 && color3 != 0&& color4 != 0&& color5 != 0&& color6 != 0&& color7 != 0&& color8 != 0&& color9 != 0 && winner == ""){
      System.out.println("Draw!");
      gameOver = true;
       jTextPane1.setText("Draw!");
  }
}

Context

StackExchange Code Review Q#151181, answer score: 2

Revisions (0)

No revisions yet.