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

Oh ye beloved bejeweled

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

Problem

Before @bazola asked his recent question, we chatted about it for a bit. I figured that I could try implementing something similar myself.

As I've heard that Scala has all the advantages of Java, but fewer of the disadvantages, I'd figured that I could try it.

I have implemented checking for board matches, removing them, and refilling the board.

I have not yet implemented any user interaction (and I'm not asking for any help in doing so either). The program simply generates a bejeweled board, then repeatedly removes all the matches and refills the board until no matches remain. In a real bejeweled game, this could be used to set up an initial board.

In the code, I have added output for each step and a pause (by reading input) just so that it's possible to see the progress.

  • Bejeweled.scala: Main program, creates the board and performs the loop of clearing and refilling



  • MatchMap.scala: The game board



  • MatchTile.scala: One tile on the game board



Code

Bejeweled.scala:

import java.util.Scanner

object Bejeweled {
    def main(args: Array[String]) {
      val map = new MatchMap(8, 8, 3);
      val sc = new Scanner(System.in)
      var cleared = false
      do {
        cleared = false
        map.print
        map.matchCheck
        map.print
        println("CLEAR")
        sc.nextLine
        while (map.clear) {
          map.print
          println("CLEAR 2")
          sc.nextLine
          cleared = true
        }
      }
      while (cleared)

      map print
    }

}


MatchMap.scala:

```
import scala.collection.generic.CanBuildFrom
import java.util.{List, Random, LinkedList, ArrayList}

class MatchMap(xsize: Integer, ysize: Integer, matchCount: Integer) {

def width = xsize
def height = ysize
def matches = matchCount
def uniqueTiles = 5

val grid = Array.ofDimMatchTile
val random = new Random()
init()
generate()

def init() {
for (x row.count(tile => tile fall) > 0) > 0
}

def

Solution

Notes

-
Although I was tempted to seek a purely functional solution to the Bejeweled problem, I realized that in doing so, your code would become mostly unrecognizable (removing mutability would change the underlying class structures and algorithms) and you wouldn't gain much benefit from the refactoring. So I kept the logic of your program the same and just refactored for conciseness and idiomatic Scala style.

-
In the methods init and generate you create a range inclusively but subtract 1 from the upper limit of the range. Instead of subtracting 1 just build your range exclusively like so:

// do this
for (x <- 0 until width) 
// not this
for (x <- 0 to width - 1)


-
In your original code there were a couple of places where you would define a function as an Integer or Boolean, instead just assign the value or expression result to a val.

// do this
val width = xsize
// not this
def width = xsize


-
In Scala you generally never have to use a return statement. In your original code you use them in the MatchTile method fall. I've reformatted that method such that the return statements are no longer necessary.

-
As far as I could tell the methods matchCheck and checkArray don't need to return a value (originally you return a Boolean from them). I've refactored the code changing the return type to Unit, the Scala equivalent to void.

-
I refactored the toString method in MatchMap to make it considerably more concise by utilizing mkString. Also instead of hard-coding a separator ("--------") you could make the separator dependent on width ("-" * width). Also, with the changes to toString I was able to remove the print() method that you had.

-
Removed boilerplate class field initialization by converting MatchMap to a case class and by prefixing val to the parameters of MatchTile. Parameters of a case class automatically become class fields. You can tell the compiler that the parameters of a (non case) class are supposed to be fields by prefixing them with val. Examples:

// do this
case class MatchMap(w: Int, h: Int) { ... }
// or this
class MatchMap(val w: Int, val h: Int) { ... }
// not this
class MatchMap(w: Int, h: Int) {
  val width = w
  val height = h
}


-
I made the initialization of grid in MatchMap more concise by using the tabulate method, allowing me to remove generate() and init().

  • Note that in #7 the integer parameters are declared with Int and not Integer.



  • There were one or two places where a semicolon slipped in where they weren't needed.



CODE

case class MatchMap(width: Int, height: Int, matches: Int) {
    val uniqueTiles = 5
    val rand = new Random()
    val grid =
      Vector.tabulate(height, width)((y, x) =>
        new MatchTile(this, y, x, rand.nextInt(uniqueTiles) + 1))

    def clear(): Boolean =
      grid.reverse.count(row =>
        row.count(tile =>
          tile.fall) > 0) > 0

    def removeValues(row: Vector[MatchTile], index: Int, count: Int): Unit =
      for (i = matches)
            removeValues(row, i, count)
          checkFor = curr
          count = 1
        }
        else
          count = count + 1
      }
    }

    def matchCheck(): Unit = {
      grid.reverse.foreach(row => checkArray(row))
      grid.transpose.foreach(row => checkArray(row))
    }

    override def toString = {
      ("-" * width) + "\n" +
      (grid map (row => row.mkString)).mkString("\n")
    }
  }

  class MatchTile(val m: MatchMap, val x: Int, val y: Int, var value: Int) {

    def clear() =
      value = -Math.abs(value)

    val isTopRow = y == 0

    def fall: Boolean = value match {
      case _ if value 
        if (isTopRow) {
          random()
          false
        } else {
          value = m.grid(y - 1)(x).value
          m.grid(y - 1)(x).clear()
          true
        }
      case _ =>
        false
    }

    def random() =
      value = m.rand.nextInt(m.uniqueTiles) + 1

    override def toString =
      if (value < 0) "_"
      else s"$value"
  }

Code Snippets

// do this
for (x <- 0 until width) 
// not this
for (x <- 0 to width - 1)
// do this
val width = xsize
// not this
def width = xsize
// do this
case class MatchMap(w: Int, h: Int) { ... }
// or this
class MatchMap(val w: Int, val h: Int) { ... }
// not this
class MatchMap(w: Int, h: Int) {
  val width = w
  val height = h
}
case class MatchMap(width: Int, height: Int, matches: Int) {
    val uniqueTiles = 5
    val rand = new Random()
    val grid =
      Vector.tabulate(height, width)((y, x) =>
        new MatchTile(this, y, x, rand.nextInt(uniqueTiles) + 1))

    def clear(): Boolean =
      grid.reverse.count(row =>
        row.count(tile =>
          tile.fall) > 0) > 0

    def removeValues(row: Vector[MatchTile], index: Int, count: Int): Unit =
      for (i <- 1 to count)
        row(index - i).clear()

    def checkArray(row: Vector[MatchTile]): Unit = {
      var checkFor = row(0).value
      var count = 0

      for (i <- 0 until row.length) {
        val curr = Math.abs(row(i).value)
        if (checkFor != curr) {
          if (count >= matches)
            removeValues(row, i, count)
          checkFor = curr
          count = 1
        }
        else
          count = count + 1
      }
    }

    def matchCheck(): Unit = {
      grid.reverse.foreach(row => checkArray(row))
      grid.transpose.foreach(row => checkArray(row))
    }

    override def toString = {
      ("-" * width) + "\n" +
      (grid map (row => row.mkString)).mkString("\n")
    }
  }

  class MatchTile(val m: MatchMap, val x: Int, val y: Int, var value: Int) {

    def clear() =
      value = -Math.abs(value)

    val isTopRow = y == 0

    def fall: Boolean = value match {
      case _ if value < 0 =>
        if (isTopRow) {
          random()
          false
        } else {
          value = m.grid(y - 1)(x).value
          m.grid(y - 1)(x).clear()
          true
        }
      case _ =>
        false
    }

    def random() =
      value = m.rand.nextInt(m.uniqueTiles) + 1

    override def toString =
      if (value < 0) "_"
      else s"$value"
  }

Context

StackExchange Code Review Q#74707, answer score: 10

Revisions (0)

No revisions yet.