cleaner code

[craig long]

I recently completed this program assignment for my beginning java class and it works well. I passed the assignment all right but was wondering if there is anything I can do to cleanup the code? Some of it seems wordy, particularly the comparisons for a winner at the bottom lines 146 - 158. thanks

  
import java.util.*;

public class CraigLongAssignment8 {
	public static void main(String[] args) {
		// Variables
		Scanner console = new Scanner(System.in);
		char[][] board = new char[3][3];
		Random rand = new Random();
		boolean turn = true;	// true for user's turn
		char uChar = ' ';
		char cChar = ' ';
		char winner = ' ';
		
		// Initialize board
		for (int i = 0; i < board.length; i++) {
			for (int j = 0; j < board[i].length; j++) {
				board[i][j] = ' ';
			}
		}
		
		// Choose user character
		System.out.print("Choose your character: ");
		do {
			String s = console.nextLine();
			if (s.length() > 1) {
				System.out.println("One character only.");
				System.out.print("Choose your character: ");
			} else {
				uChar = Character.toUpperCase(s.charAt(0));
			}
			if (!(uChar == 'X' || uChar == 'O')) {
				System.out.println("Must be an 'X' or 'O'.");
				System.out.print("Choose your character: ");
			}
		} while (!(uChar == 'X' ||uChar == 'O'));
		
		// Set computer char
		if (uChar == 'X')
			cChar = 'O';
		else
			cChar = 'X';
		
		// Determine who goes first
		if (rand.nextBoolean()) {
			System.out.println("You go first.");
		}
		else {
			System.out.println("You go second.");
			turn = false;
		}
		
		// Run game
		do {
			// User's turn
			if (turn) {
				// Variables for user coordinates
				int x = -1;
				int y = -1;
				
				do {
					System.out.print("Enter column coordinate: ");
					do {
// 						if (!console.hasNextInt()) {
// 							System.out.println("Numbers only.");
// 							System.out.print("Enter column coordinates: ");
// 							continue;
// 						}
						
						x = console.nextInt();
						
						if (x < 1 || x > 3) {
							System.out.println("Only values 1-3 are accepted.");
							System.out.print("Enter column coordinate: ");
						}
					} while (x < 1 || x > 3);
					
					System.out.print("Enter row coordinate: ");
					do {
// 						while (!console.hasNextInt()) {
// 							System.out.println("Numbers only.");
// 							System.out.println("Enter row coordinate: ");
// 						}
						
						y = console.nextInt();
						
						if (y < 1 || y > 3) {
							System.out.println("Only values 1-3 are accepted.");
							System.out.print("Enter row coordinate: ");
						}
					} while (y < 1 || y > 3);
					
					//System.out.print("Enter coordinates: ");
					x--;
					y--;
					
					if (board[x][y] == ' ') {
						board[x][y] = uChar;
					}
					else {
						System.out.println("That square is already filled.");
					}
				} while (!(board[x][y] == uChar));
				
				System.out.println();
				
				// Computer's turn
				turn = false;
			}
			else {
				int x = -1;
				int y = -1;
				boolean keepGoing = true;
				
				do {
					x = rand.nextInt(3);
					y = rand.nextInt(3);
					
					if (board[x][y] == ' ') {
						System.out.println("Computer coords: " + (x + 1) + " " + (y + 1));
						System.out.println();
						board[x][y] = cChar;
						keepGoing = false;
					}
				} while (keepGoing);
				
				// User's turn
				turn = true;
			}
			
			// Print out current board
			for (int y = 0; y < board.length; y++) {
					for (int x = 0; x < board.length; x++) {
					System.out.print(board[x][y]);
					if (x < 2) {
						System.out.print("|");
					}
				}
				System.out.println();
				if (y < board.length - 1) {
					System.out.println("-+-+-");
				}
			}
			System.out.println();
			
			for (int i = 0; i < 3; i++) {
				if (board[i][0] != ' ' && board[i][0] == board[i][1] && board[i][1] == board[i][2]) {
					winner = board[i][0];
				}
			}
			for (int i = 0; i < 3; i++) {
				if (board[0][i] != ' ' && board[0][i] == board[1][i] && board[1][i] == board[2][i]) {
					winner = board[0][i];
				}
			}
			if ((board[0][0] == board[1][1] && board[1][1] == board[2][2]) ||
				(board[0][2] == board[1][1] && board[1][1] == board[2][0])) {
				winner = board[1][1];
			}
			if (winner != ' ') {
				System.out.println("Winner: " + winner);
			}
			if (winner == uChar) {
				System.out.println("You win!");
			}
			if (winner == cChar) {
				System.out.println("You lose!");
			}
		} while (winner == ' ');
	}
}

[Ernest Friedman-Hill]

The first thing I would do to clean this up would be to move the major data structures (e.g., "board") from being local variables to member variables, and then start breaking the one long method up into a number of much smaller, single-purpose routines. As a start, consider that every where you've written a comment like "initialize board", you could instead have a call to a method named "initializeBoard()" which did just that one thing. Sometimes these methods would operate on shared data like "board", while other times they'd communicate with main() using arguments and return values.

Once the code is broken into these small chunks, then you can consider which ones need the most work to make them easy to understand.

[Garrett Rowe]

Clean code is a particularly subjective phrase, however looking at what you have, the first thing I would advise would be to decompose your singel main method into smaller methods. These methods should do exactly one thing, and return a result. If you start down that road, your main method might begin to look something like this:

public static void main(String[] args) {
   char[][] board = initalizeBoard();
   String userCharacter = chooseUserCharacter();
   String cpuCharacter = chooseCPUCharacter();
   while (!gameIsOver()) {
      playGame(board, userCharacter, cpuCharacter);
   }
}

Once you start decomposing your program into smaller units, you usually end up with methods that are smaller, and easier to understand (and test).

Edit: Ernest beat me to the punch!