Sorry for the ridiculously long answer. I've added an (in my opinion) better way of doing this at the very bottom.
Here are a few things you could improve:
class TicTacToe
There's no need for this class. As far as I can tell, you can get rid of it. Actually, as Simon Forsberg said in the comments, a class could be useful for having multiple instances of the game running at once. However, you'll have to move at least the startGame method into the class for that to happen. You can also make the class stateful and expose modified versions of methods such as playRound if you want to. I'll add a class to my code at the bottom later.
Spaces
Overall, your code is well-formatted, but there a few instances like these, where you missed spaces:
if(turnNumber%2 == 0) |> if (turnNumber % 2 == 0)
pattern=>{ |> pattern => {
def startGame: Unit ={ |> def startGame(): Unit = {
Type aliases
I don't know about you, but I like using type aliases because they help me remember what each type represents. It's also handy when you have to refactor your code, e.g., if you want to represent moves using tuples representing the row and column ((Int, Int)) or you want to make a Board class instead of just using an array of characters, you don't have to change your method signatures - they can still return and accept objects of type Player and Board.
type Player = Char
type Board = Array[Player]
Unnecessary braces and the main method
Instead of
def main(args: Array[String]): Unit = {
startGame
}
you could make it a one-liner
def main(args: Array[String]) = startGame
However, it'd be much more helpful to announce instructions before starting the game (I know those instructions don't match your own game, but bear with me).
def main(args: Array[String]): Unit = {
println("Welcome to Tic Tac Toe!")
println("To play, enter the row and column of the cell where you want to move when prompted")
println("Both the row and column must be numbers from 1 to 3")
runGame()
}
startGame
Rather than startGame, I feel like you should name it runGame, but that's entirely subjective, and you should pick whatever feels more intuitive to you.
More importantly, I think the startGame should be a nilary method rather than a nullary method, i.e., it should have an empty parameter list so that it looks like a proper method call rather than a property access. Currently, it looks very confusing when you just have startGame to run the entire game, since it looks like an unused expression.
Storing the board
Using a 1-D array of characters to represent a board is fine for now, although not very good functional programming style. There are a lot of other issues here, though.
board.update
You can use board(atPosition) = nextPlayer(board) instead of board.update(atPosition, nextPlayer(board)), since the update method is one of Scala's special methods that let you use syntactic sugar.
Adding the current board to the game
Currently, you use List(board) ++ game, which creates a new list and then concatenates game to it. A better approach would be board :: game, which is more idiomatic and simpler.
Why using a List[Array[Char]] is bad
First of all, there is absolutely no reason to maintain a list of all the past boards. You don't use the game variable everywhere. You can just have a single Array[Char] to keep track of the current board. Even if you do need to be able to go back to a previous move, you can just maintain a List[Move] and use it to reconstruct the board at a specific time.
Second, every single board you store in game refers to the exact same object! You're not copying board anywhere, so when you write the below code, you're getting the head of game, updating it, and then prepending that same board back onto game.
val board: Array[Char] = game.head
board.update(atPosition, nextPlayer(board))
A better solution would be to use tuples or a case class of your making. I'm going to just use tuples for now, because case classes would bloat the code.
type Row = (Player, Player, Player)
type Board = (Row, Row, Row)
Now that the board's 2-D, let's also make our moves 2-D, actually. They'll represent the row and column of each move. I made them 1-indexed in my code because I also make the user pick a number from 1 to 3 instead of 0 to 2 because I feel it'd be easier. By also internally using 1 instead of 0, we'll reduce off-by-one errors.
type Move = (Int, Int)
Variables
At the top of your object, you have a ton of variables, of which you need only one - patterns (which I'm going to rename to winCases, because that is more descriptive to me). There's no need to make separate public variables for l1, l2, etc. You can just do it like this (I used Set because the order doesn't matter):
val winCases = Set(
Set((1, 1), (1, 2), (1, 3)),
Set((2, 1), (2, 2), (2, 3)),
Set((3, 1), (3, 2), (3, 3)),
Set((1, 1), (2, 1), (3, 1)),
Set((1, 2), (2, 2), (3, 2)),
Set((1, 3), (2, 3), (3, 3)),
Set((1, 1), (2, 2), (3, 3)),
Set((1, 3), (2, 2), (3, 1)),
)
startBoard can just be a local variable in runGame and doesn't have to be accessible by everyone.
winConditions we won't need because the whole isWon method can be refactored. There's absolutely no need to make a new string for each element of patterns/winCases.
Finding the winner
I don't like the fact that your isWon method prints the winner instead of only returning whether or not someone has won the game. A better way to do it would be to return a player and let the calling method decide what to display to the user. For that, I made this method:
def findWinner(board: Board): Option[Option[Player]] =
if (isWinner(player1, board)) Some(Some(player1))
else if (isWinner(player2, board)) Some(Some(player2))
else if (isTie(board)) Some(None)
else None
Having an Option[Option[Player]] lets us encode multiple things into that single value. If it's a None, we know the game will continue. If it's a Some, the game's ended. If it's the latter, containing a Some, there's a winner, and if it contains a None, there's a tie.
Back to startGame/runGame
Among other things, I've renamed the playGameAt function to playRound. I also changed the signature. It takes the current board and player, and outputs the winner of the game. If there's a winner, it's a Some. If there's a tie, it's a None.
def playRound(board: Board, curr: Player): Option[Player]
Here's how the new function looks:
@tailrec
def playRound(board: Board, curr: Player): Option[Player] = {
printBoard(board)
println(s"Player $curr's move")
val move = nextMove(board)
val newBoard = moveTo(curr, move, board)
findWinner(newBoard) match {
case Some(possWinner) => possWinner
case None => playRound(newBoard, nextPlayer(curr))
}
}
I've put the call to printBoard at the very top, because even though all tic-tac-toe boards are the same (unless you're playing a special variant), I personally would like to see the board I'm going to be moving on before I actually select a move. It's totally subjective, but I prefer it this way.
The print statement lets you know which player is supposed to move. The way you have it now, the users have to scroll up to see whose move it is currently, and this seems more helpful.
After that, it gets the next move with the nextMove function (which I'll put in later), creates a new board using that move, and tries to find the winner (see above for how the findWinner method works). If the game's ended, it returns the winner (or None if it's a tie). If not, it plays another round using a new board and the other player.
nextPlayer is implemented like this, by the way:
def nextPlayer(curr: Player): Player =
if (curr == player1) player2
else player1
The entire runGame function looks like this:
def runGame() = {
@tailrec
def playRound(board: Board, curr: Player): Option[Player] = ...
val startBoard = (
(default, default, default),
(default, default, default),
(default, default, default)
)
val winner = playRound(startBoard, player1)
winner match {
case Some(player) => println(s"Player $player won!")
case None => println("Tie")
}
}
Here, startBoard is just a local variable, because I don't think there's any reason for anybody outside this method to know about it. default is a Char that represents a cell where no one's moved. player1 and player2 are used to mark where Player 1 and Player 2 moved, respectively.
val default = ' '
val player1 = 'x'
val player2 = 'o'
I also moved the print statement from isWon to here, so that there could be a customized message. Otherwise, if there was a tie, nothing would happen.
User input
It doesn't make sense to me that you're using a JOptionPane and displaying everything in the terminal. Why not make the user input also come from the console? We can write our nextMove method like this:
@tailrec
def nextMove(board: Board): Move = {
val move = (nextRowOrCol("Row"), nextRowOrCol("Column"))
if (isValid(move, board)) {
move
} else {
println("That move is already taken. Please enter a different move.")
nextMove(board)
}
}
The above code reads a row and column using the helper function nextRowOrCol, then checks if it's a valid move, i.e., no one's moved there already. If it is, it just returns it, and if not, it gets the user to re-enter a move. I'll add the isValid function a little below.
nextRowOrCol is implemented like this (prompt can be either "Row" or "Column"). It uses regex to ensure that the input is a number between 1 and 3. Like the nextMove function, if the row or column inputted is valid, it returns it directly, otherwise, it prompts the user(s) again.
private def nextRowOrCol(prompt: String): Int = {
val input = readLine(s"$prompt: ")
if (input.matches("[1-3]")) {
input.toInt
} else {
println("Please enter a number from 1 to 3")
nextRowOrCol(prompt)
}
}
The entire code
As the heading says, here's the entire code. You'll notice that it's wayyy longer than what you have currently, mostly because I used tuples, which complicated everything, but also because your original solution didn't have a lot of functionality.
I'm sure you can find a way to make it shorter, especially if you use your own case classes to represent everything. I've made almost every function that deals with tiny details like destructuring tuples private, but the public functions shouldn't need to be changed much even if you do decide to make a case class Board or something like that.
import scala.io.StdIn.readLine
import scala.annotation.tailrec
object TicTacToe {
type Player = Char
type Move = (Int, Int)
type Row = (Player, Player, Player)
type Board = (Row, Row, Row)
val winCases = Set(
Set((1, 1), (1, 2), (1, 3)),
Set((2, 1), (2, 2), (2, 3)),
Set((3, 1), (3, 2), (3, 3)),
Set((1, 1), (2, 1), (3, 1)),
Set((1, 2), (2, 2), (3, 2)),
Set((1, 3), (2, 3), (3, 3)),
Set((1, 1), (2, 2), (3, 3)),
Set((1, 3), (2, 2), (3, 1)),
)
val default = ' '
val player1 = 'x'
val player2 = 'o'
def main(args: Array[String]) = {
println("Welcome to TicTacToe!")
println("To play, enter the row and column of the cell where you want to move when prompted")
println("Both the row and column must be numbers from 1 to 3")
runGame()
}
def runGame() = {
@tailrec
def playRound(board: Board, curr: Player): Option[Player] = {
printBoard(board)
println(s"Player $curr's move")
val move = nextMove(board)
val newBoard = moveTo(curr, move, board)
findWinner(newBoard) match {
case Some(possWinner) => possWinner
case None => playRound(newBoard, nextPlayer(curr))
}
}
val startBoard = (
(default, default, default),
(default, default, default),
(default, default, default)
)
val winner = playRound(startBoard, player1)
winner match {
case Some(player) => println(s"Player $player won!")
case None => println("Tie")
}
}
def findWinner(board: Board): Option[Option[Player]] =
if (isWinner(player1, board)) Some(Some(player1))
else if (isWinner(player2, board)) Some(Some(player2))
else if (isTie(board)) Some(None)
else None
def moveTo(player: Player, move: Move, board: Board): Board = {
val (row0, row1, row2) = board
val (r, c) = move
def updateTuple[T](tup: (T, T, T), ind: Int)(f: T => T): (T, T, T) =
ind match {
case 1 => tup.copy(_1 = f(tup._1))
case 2 => tup.copy(_2 = f(tup._2))
case 3 => tup.copy(_3 = f(tup._3))
}
updateTuple(board, r) {
row => updateTuple(row, c)(_ => player)
}
}
def isWinner(player: Player, board: Board): Boolean =
winCases.exists(winCase =>
winCase.forall(move => playerAt(move, board) == player)
)
def isTie(board: Board): Boolean = !board.productIterator.exists {
row => row.asInstanceOf[Row].productIterator.contains(default)
}
def playerAt(move: Move, board: Board): Player = {
val (r, c) = move
elementAt(elementAt(board, r), c)
}
private def elementAt[T](tup: (T, T, T), ind: Int): T =
ind match {
case 1 => tup._1
case 2 => tup._2
case 3 => tup._3
}
@tailrec
def nextMove(board: Board): Move = {
val move = (nextRowOrCol("Row"), nextRowOrCol("Column"))
if (isValid(move, board)) {
move
} else {
println("That move is already taken. Please enter a different move.")
nextMove(board)
}
}
private def nextRowOrCol(prompt: String): Int = {
val input = readLine(s"$prompt: ")
if (input.matches("[1-3]")) {
input.toInt
} else {
println("Please enter a number from 1 to 3")
nextRowOrCol(prompt)
}
}
def isValid(move: Move, board: Board): Boolean =
playerAt(move, board) == default
def nextPlayer(curr: Player): Player =
if (curr == player1) player2
else player1
def printBoard(board: Board): Unit =
print(
"__________________\n" +
tup2String(
mapTuple(board) {row => tup2String(row, "|")},
"------\n"
)
)
private def tup2String[T](tup: (T, T, T), sep: String): String =
s"${tup._1}$sep${tup._2}$sep${tup._3}\n"
private def mapTuple[T, R](tup: (T, T, T))(f: T => R): (R, R, R) =
(f(tup._1), f(tup._2), f(tup._3))
}
An alternative way of storing the board
Working with tuples is really annoying, and even with case classes, you'd have to define your own methods and stuff. A nicer way to store the board would be to just maintain a list of all the moves anyone's ever made. Every move should contain where that move was made and the player who made that move, so let's makes these two types. By the way, the Move from before is more like Coord here.
type Coord = (Int, Int)
type Move = (Coord, Player)
Now everywhere we use board: Board, we just replace that with moves: List[Move].
playRound doesn't change all that much. The variable move now has to include the current player because of how we defined Move above, and the newMoves variable (analogous to newBoard) is constructed by prepending moves to the pre-existing list of moves, which is a lot easier than creating a moveTo function that does all sorts of crazy stuff inside. Everywhere else, just remember that board has been replaced with moves.
@tailrec
def playRound(moves: List[Move], curr: Player): Option[Player] = {
println(s"Player $curr's move")
val move = (nextMove(moves), curr)
val newMoves = move :: moves
printBoard(newMoves)
findWinner(newMoves) match {
case Some(possWinner) => possWinner
case None => playRound(newMoves, nextPlayer(curr))
}
}
runGame only has 1 change: instead of manually creating a variable called startBoard filled with the default character (' '), you can use List.empty (or Nil):
val winner = playRound(List.empty, player1)
playerAt is a lot simpler now. It tries to find a move with the given coordinates, and if no move with those coordinates is found in our List[Move], then default is chosen.
def playerAt(coord: Coord, moves: List[Move]): Player =
moves.find(move => move._1 == coord).map(_._2).getOrElse(default)
isTie is also a lot simpler - just check if 9 moves have been made!
def isTie(moves: List[Move]): Boolean = moves.size == 9
printBoard is the only one with big-ish changes (the good kind). You can just use map and mkString now that we're not using tuples.
def printBoard(moves: List[Move]): Unit =
print(
1 to 3 map { r =>
1 to 3 map { c =>
playerAt((r, c), moves)
} mkString "|"
} mkString ("__________\n", "\n------\n", "\n")
)
The entire code (note that I've used two spaces to indent here, since that appears to be more commonly used in the Scala world, and it's what scalafmt uses):
import scala.io.StdIn.readLine
import scala.annotation.tailrec
object TicTacToe2 {
type Player = Char
type Coord = (Int, Int)
type Move = (Coord, Player)
val winCases: Set[Set[Coord]] = Set(
Set((1, 1), (1, 2), (1, 3)),
Set((2, 1), (2, 2), (2, 3)),
Set((3, 1), (3, 2), (3, 3)),
Set((1, 1), (2, 1), (3, 1)),
Set((1, 2), (2, 2), (3, 2)),
Set((1, 3), (2, 3), (3, 3)),
Set((1, 1), (2, 2), (3, 3)),
Set((1, 3), (2, 2), (3, 1))
)
val default = ' '
val player1 = 'x'
val player2 = 'o'
def main(args: Array[String]) = {
println("Welcome to TicTacToe!")
println(
"To play, enter the row and column of the cell where you want to move when prompted"
)
println("Both the row and column must be numbers from 1 to 3")
printBoard(List.empty)
runGame()
}
def runGame() = {
@tailrec
def playRound(moves: List[Move], curr: Player): Option[Player] = {
println(s"Player $curr's move")
val move = (nextMove(moves), curr)
val newMoves = move :: moves
printBoard(newMoves)
findWinner(newMoves) match {
case Some(possWinner) => possWinner
case None => playRound(newMoves, nextPlayer(curr))
}
}
val winner = playRound(List.empty, player1)
winner match {
case Some(player) => println(s"Player $player won!")
case None => println("Tie")
}
}
def findWinner(board: Board): Option[Option[Player]] =
if (isWinner(player1, board)) Some(Some(player1))
else if (isWinner(player2, board)) Some(Some(player2))
else if (isTie(board)) Some(None)
else None
def isWinner(player: Player, moves: List[Move]): Boolean =
winCases.exists { winCase =>
winCase.forall(move => playerAt(move, moves) == player)
}
def isTie(moves: List[Move]): Boolean = moves.size == 9
def playerAt(coord: Coord, moves: List[Move]): Player =
moves.find(move => move._1 == coord).map(_._2).getOrElse(default)
@tailrec
def nextMove(moves: List[Move]): Coord = {
val coord = (nextRowOrCol("Row"), nextRowOrCol("Column"))
if (isValid(coord, moves)) {
coord
} else {
println("That move is already taken. Please enter a different move.")
nextMove(moves)
}
}
private def nextRowOrCol(prompt: String): Int = {
val input = readLine(s"$prompt: ")
if (input.matches("[1-3]")) {
input.toInt
} else {
println("Please enter a number from 1 to 3")
nextRowOrCol(prompt)
}
}
def isValid(coord: Coord, moves: List[Move]): Boolean =
playerAt(coord, moves) == default
def nextPlayer(curr: Player): Player =
if (curr == player1) player2
else player1
def printBoard(moves: List[Move]): Unit =
print(
1 to 3 map { r =>
1 to 3 map { c =>
playerAt((r, c), moves)
} mkString "|"
} mkString ("__________\n", "\n------\n", "\n")
)
}
By the way, here's a small change you can make concerning the isValid method. Instead of returning a boolean, return an Option with which you can do getOrElse. If you write a validate function like this:
def validate(coord: Coord, moves: List[Move]): Option[Coord] =
Option.when(playerAt(coord, moves) == default)(coord)
you can use it in nextMove like this, which looks much more idiomatic. The only thing is that you'll have to drop the tailrec annotation.
def nextMove(moves: List[Move]): Coord = {
val coord = (nextRowOrCol("Row"), nextRowOrCol("Column"))
validate(coord, moves).getOrElse {
println("That move is already taken. Please enter a different move.")
nextMove(moves)
}
}
Let me know if there's anything I did wrong or can improve.
