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

Connect Four In Idiomatic Scala

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

Problem

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 (_  x == Markers.Empty)

    def isInBounds(index: Index) = {
      val row = index.row
      val col = index.col 
      row >= 0 && row = 0 && col  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  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) 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

}

Solution

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



  • 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.

Context

StackExchange Code Review Q#12312, answer score: 10

Revisions (0)

No revisions yet.