5
\$\begingroup\$

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

}
\$\endgroup\$

1 Answer 1

10
\$\begingroup\$

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.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.