Connect Four In Idiomatic Scala

Question

Learning Scala

I'm trying to learn Scala and I'm interested in criticism about how to make my code more idiomatic. I've programmed a simple Connect-four game. Can I get some feedback?

https://github.com/jamiely/connect-four-scala

The two most important classes are Board.scala and Game.scala. They are shown below for your convenience as well.

Board.scala

package connect_four
import connect_four._
class Index(val row: Int, val col: Int) {
 def tuple(): (Int, Int) = (row, col)
}
class Size(val width: Int = 7, val height: Int = 6)
class Board(val size: Size = new Size()) {
 val length = size.width * size.height
 val board = (for (_ <- 0 to length-1) yield Markers.Empty).toArray
 // Return a list of index objects
 def getPositionIndices() = {
 for {
 row <- 0 to size.height-1
 col <- 0 to size.width-1
 }
 yield new Index(row, col)
 }
 def isEmpty = board.forall(x => x == Markers.Empty)
 def isInBounds(index: Index) = {
 val row = index.row
 val col = index.col 
 row >= 0 && row < size.height && col >= 0 && col < size.width 
 }
 def fromIndex(index: Index): Option[Int] = {
 if(isInBounds(index)) {
 Some(index.row * size.width + index.col)
 }
 else None
 }
 def move(marker: Markers.Marker, index: Index): Boolean = {
 val pos: Option[Int] = for {
 pos <- fromIndex(index)
 _ <- Some(updatePosition(marker, pos))
 } yield pos
 !pos.isEmpty
 }
 // Updates the given position without performing a check. 
 // @returns Returns a pair of the marker that was put at the position and the position.
 def updatePosition(marker: Markers.Marker, position: Int) = {
 board(position) = marker
 (marker, position)
 }
 def markerAt(index: Index): Option[Markers.Marker] = {
 for {
 pos <- fromIndex(index)
 m <- Some(board(pos))
 } yield m
 }
 def posIs(marker: Markers.Marker, index: Index) = {
 val result: Option[Boolean] = for {
 m <- markerAt(index)
 // return the result of a check against the marker
 result <- Some(m == marker)
 } yield result
 result.getOrElse(false)
 }
 def hasMovesLeft = board.exists(x => x == Markers.Empty)
}

Game.scala

package connect_four
import connect_four._
class Game {
 val board = new Board
 var currentMarker = Markers.A
 // directions are deltas used to check board locations in the cardinal directions
 val directions = (for {
 i <- -1 to 1 
 j <- -1 to 1
 if !(i==0 && j==0)
 } yield (i, j)).toList
 def checkPosition(index:Index, marker:Markers.Marker, delta: (Int, Int), steps: Int): Boolean = {
 if(steps == 0) {
 true
 }
 else if (board.isInBounds(index) && board.posIs(marker, index)) {
 val newIndex = new Index(index.row + delta._1, index.col + delta._2)
 checkPosition(newIndex, marker, delta, steps-1)
 }
 else {
 false
 } 
 }
 def isWin:Boolean = 
 board.getPositionIndices().exists(testWinAtIndex)
 def testWinAtIndex(index:Index): Boolean = {
 board.markerAt(index) match {
 case None => false
 case Some(Markers.Empty) => false
 case Some(m) => directions.exists(delta => checkPosition(index, m, delta, 4)) 
 }
 }
 def getFirstEmptyRowInColumn(c: Int): Option[Int] = {
 def helper(row: Int): Int = {
 if (row < 0) {
 row
 } else {
 val marker = markerAt(new Index(row, c))
 if (marker == Some(Markers.Empty)) row else helper(row - 1)
 }
 }
 if(!board.isInBounds(new Index(0, c))) {
 None
 }
 else {
 val r = helper(board.size.height-1)
 if(r < 0) None else Some(r)
 }
 } 
 def toggleMarker(): Markers.Marker = {
 currentMarker = if(currentMarker == Markers.A) Markers.B else Markers.A
 currentMarker
 }
 def updateBoard(index: Index): Markers.Marker = {
 board.move(currentMarker, index)
 toggleMarker()
 }
 def move(mv: Move): Option[Markers.Marker] = {
 if(isWin) {
 None
 }
 else {
 for {
 row <- getFirstEmptyRowInColumn(mv.col)
 result <- if(row >= 0) Some(updateBoard(new Index(row, mv.col))) else None
 } yield result
 }
 }
 def markerAt(index:Index): Option[Markers.Marker] = board.markerAt(index)
 def getCurrentMarker: Markers.Marker = currentMarker
}

Answer 1

Nice code! You did the most things correct only some hints:

• Use case classes. They bring you some nice methods (like copy or apply) and result in easier maintainable code. There is no need any more to instantiate objects directly with new - instead it is possible to use the apply-method (factory pattern). Later, when your application grows, maybe you detect that you have to much object creations - with case classes it is easy to cache object instantiation inside of the apply-method - without changing any call side code.
• If you need mutable state then centralize and encapsulate it in single places. The Game class does have some public var - refactor this. No one outside of this class should be able to change the state of this var.
• Make Game immutable. You should hold different game instances at the call side such as your UIConsole.
• In Scala algebraic data types are more powerful than enumerations.
• Instead of sharing libraries in your repository use build tools like sbt.
• Move test code outside of your main code - this makes it easier to publish your code as an standalone application.

Syntax clues:

• You can leave curly braces when you have single expressions.
• For-expressions can be nested: for(a <- ...; b <- ...; c <- ...)
• Methods with side effects should marked with parentheses: def x { println } -> def x() { println }
• Type Application is deprecated, use App instead
• There is no need to move each class to its own file. In particular when you have a lot of small classes (like your Move, Markers) it can be more clear to put them together.

Some months ago I have written the game Othello in Scala - here is the result. Maybe it offers some concepts to you.