Criticizing My Scala Code

I am Scala n00b (but I am familiar with other languages) and I learn the language when I find the time - I still enjoy it very much!

Usually, when I learn a new language, the first thing I do is implement the Conway Game of Life , because it is complex enough to give a good sense of the language, but small enough to be able to whip in a couple of hours (most of which devoted to the fight against syntax).

Anyhoo, having completed this exercise with Scala, I was hoping that Scala gurus could take a look at the code that I have run out and provide him with feedback. I am for something - algorithmic improvements (especially parallel solutions!), Stylistic improvements, alternative APIs or language constructs, aversion to the length of the names of my functions - no matter what you have, I really want to hear it!

You should be able to run the following script through scala GameOfLife.scala- by default it will run a 20x20 board with one glider on it - feel free to experiment.

// CONWAY GAME OF LIFE (SCALA)
abstract class GameOfLifeBoard(val aliveCells : Set[Tuple2[Int, Int]])
{
  // Executes a "time tick" - returns a new board containing the next generation
  def tick : GameOfLifeBoard

  // Is the board empty?
  def empty : Boolean = aliveCells.size == 0

  // Is the given cell alive?
  protected def alive(cell : Tuple2[Int, Int]) : Boolean = aliveCells contains cell

  // Is the given cell dead?
  protected def dead(cell : Tuple2[Int, Int]) : Boolean = !alive(cell)

}


class InfiniteGameOfLifeBoard(aliveCells : Set[Tuple2[Int, Int]])
  extends GameOfLifeBoard(aliveCells)
{
  // Executes a "time tick" - returns a new board containing the next generation
  override def tick : GameOfLifeBoard = new InfiniteGameOfLifeBoard(nextGeneration)

  // The next generation of this board
  protected def nextGeneration : Set[Tuple2[Int, Int]] = aliveCells flatMap neighbours filter shouldCellLiveInNextGeneration

  // Should the given cell should live in the next generation?
  protected def shouldCellLiveInNextGeneration(cell : Tuple2[Int, Int]) : Boolean = (alive(cell) && (numberOfAliveNeighbours(cell) == 2 || numberOfAliveNeighbours(cell) == 3)) ||
                                                                                    (dead(cell)  && numberOfAliveNeighbours(cell) == 3)

  // The number of alive neighbours for the given cell
  protected def numberOfAliveNeighbours(cell : Tuple2[Int, Int]) : Int = aliveNeighbours(cell) size

  // Returns the alive neighbours for the given cell
  protected def aliveNeighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = aliveCells intersect neighbours(cell)

  // Returns the coordinates of all of the neighbouring cells of the given cell
  protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = Set((cell._1-1, cell._2-1), (cell._1, cell._2-1), (cell._1+1, cell._2-1),
                                                                                  (cell._1-1, cell._2),                         (cell._1+1, cell._2),
                                                                                  (cell._1-1, cell._2+1), (cell._1, cell._2+1), (cell._1+1, cell._2+1))

  // Information on where the currently live cells are
  protected def xVals  = aliveCells map { cell => cell._1 }
  protected def xMin   = (xVals reduceLeft (_ min _)) - 1
  protected def xMax   = (xVals reduceLeft (_ max _)) + 1
  protected def xRange = xMin until xMax + 1

  protected def yVals  = aliveCells map { cell => cell._2 }
  protected def yMin   = (yVals reduceLeft (_ min _)) - 1
  protected def yMax   = (yVals reduceLeft (_ max _)) + 1
  protected def yRange = yMin until yMax + 1


  // Returns a simple graphical representation of this board
  override def toString : String =
  {
    var result = ""

    for (y <- yRange)
    {
      for (x <- xRange)
      {
        if (alive (x,y)) result += "# "
        else result += ". "
      }

      result += "\n"
    }

    result
  }

  // Equality stuff
  override def equals(other : Any) : Boolean =
  {
    other match
    {
      case that : InfiniteGameOfLifeBoard => (that canEqual this) &&
                                             that.aliveCells == this.aliveCells
      case _ => false
    }
  }

  def canEqual(other : Any) : Boolean = other.isInstanceOf[InfiniteGameOfLifeBoard]

  override def hashCode = aliveCells.hashCode

}


class FiniteGameOfLifeBoard(val boardWidth : Int, val boardHeight : Int, aliveCells : Set[Tuple2[Int, Int]])
  extends InfiniteGameOfLifeBoard(aliveCells)
{
  override def tick : GameOfLifeBoard = new FiniteGameOfLifeBoard(boardWidth, boardHeight, nextGeneration)

  // Returns the coordinates of all of the neighbouring cells of the given cell
  override protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = super.neighbours(cell) filter { cell => cell._1 >= 0 && cell._1 < boardWidth &&
                                                                                                                               cell._2 >= 0 && cell._2 < boardHeight }

  // Information on where the currently live cells are
  override protected def xRange = 0 until boardWidth
  override protected def yRange = 0 until boardHeight

  // Equality stuff
  override def equals(other : Any) : Boolean =
  {
    other match
    {
      case that : FiniteGameOfLifeBoard => (that canEqual this) &&
                                           that.boardWidth  == this.boardWidth &&
                                           that.boardHeight == this.boardHeight &&
                                           that.aliveCells  == this.aliveCells
      case _ => false
    }
  }

  override def canEqual(other : Any) : Boolean = other.isInstanceOf[FiniteGameOfLifeBoard]

  override def hashCode : Int =
  {
    41 * (
      41 * (
        41 + super.hashCode 
      ) + boardHeight.hashCode
    ) + boardWidth.hashCode
  }

}


class GameOfLife(initialBoard: GameOfLifeBoard)
{
  // Run the game of life until the board is empty or the exact same board is seen twice
  // Important note: this method does NOT necessarily terminate!!
  def go : Unit =
  {
    var currentBoard   = initialBoard
    var previousBoards = List[GameOfLifeBoard]()

    while (!currentBoard.empty && !(previousBoards contains currentBoard))
    {
      print(27.toChar + "[2J")  // ANSI: clear screen
      print(27.toChar + "[;H")  // ANSI: move cursor to top left corner of screen
      println(currentBoard.toString)
      Thread.sleep(75)

      // Warning: unbounded list concatenation can result in OutOfMemoryExceptions  ####TODO: replace with LRU bounded list
      previousBoards = List(currentBoard) ::: previousBoards
      currentBoard   = currentBoard tick
    }

    // Print the final board
    print(27.toChar + "[2J")  // ANSI: clear screen
    print(27.toChar + "[;H")  // ANSI: move cursor to top left corner of screen
    println(currentBoard.toString)
  }
}



// Script starts here
val simple = Set((1,1))
val square = Set((4,4), (4,5), (5,4), (5,5))
val glider = Set((2,1), (3,2), (1,3), (2,3), (3,3))

val initialBoard = glider

(new GameOfLife(new FiniteGameOfLifeBoard(20, 20, initialBoard))).go
//(new GameOfLife(new InfiniteGameOfLifeBoard(initialBoard))).go

// COPYRIGHT PETER MONKS 2010

: (Scala ftw!), , . - , , Scala ( , )?

+5
5

protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = 
   Set((cell._1-1, cell._2-1), (cell._1, cell._2-1), (cell._1+1, cell._2-1),
   (cell._1-1, cell._2),                         (cell._1+1, cell._2),
   (cell._1-1, cell._2+1), (cell._1, cell._2+1), (cell._1+1, cell._2+1))

:

protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = { 
  val direction = Set(-1,0,1)
  for(x <- direction; y<-direction; if(x !=0 || y != 0)) 
    yield (cell._1+x,cell._2+y)
}

var previousBoards, var currentBoard: go , vals.

while (!currentBoard.empty && !(previousBoards contains currentBoard)): Iterable InfiniteGameOfLifeBoard, for.

: , scaladoc?

+5

, "", .

+2

Tuple2 ( TupleN) parens:

type cell = (Int, Int)
// same as:
type cell = Tuple2[Int, Int]

val cell00 = (0, 0)
// same as:
val cell00 = Tuple2(0, 0)
+1

for toString , .

yRange.map(y => {
  xRange.map(x => if (alive(x,y)) "#" else ".").mkString(" ")
}).mkString("\n")

a until b+1 a to b.

P.S. (i => { - , .

+1

concurrency, . . - - .: -)

, , .

+1

All Articles