patternModerate
Oh ye beloved bejeweled
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.
Code
Bejeweled.scala:
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
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
-
In your original code there were a couple of places where you would define a function as an
-
In Scala you generally never have to use a return statement. In your original code you use them in the
-
As far as I could tell the methods
-
I refactored the
-
Removed boilerplate class field initialization by converting
-
I made the initialization of
CODE
-
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
Intand notInteger.
- 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.