Ett designbeslut kan kännas okej när man gör det, men hundra rader kod senare upptäcker man att det plötsligt ser fult ut. I mitt förra gjorde jag ett designbeslut att låta klassen Position hantera placering av Piece men inte hantera rotationsläget av den samma (det var gjort så i Java-versionen). När jag lade till hantering av den nya klassen Move till Position blev problemen tydliga. Move hanterar både x,y och rotation och det finns stöd för att sätta en Move direkt i Position via metoden movePiece(move: Move). Att det samtidigt gick att rotera en Piece via en instans utanför Positions kontroll ledde till extrakod som dessutom inte var helt uppenbar varför den behövdes när man tittade på den.
Det blev helt enkelt fult och är det fult så är det fel. Jag hade i och för sig haft en känsla hela tiden att det kanske inte var så bra att låta Piece hantera sin egen rotation, men valde ändå att testa detta då det verkade bli en enklare hantering (vilket det också var till en början). Fick det även påpekat av en kollega som ställde sig undrande till min design! Problemet löste sig genom att låta Position ha hand om både bitens placering och rotationsläge. Ett annat resultat av manövern blev att koden från PieceType fick flytta in till Piece och att PieceType kunde tas bort. Position ser nu ut så här:
package net.sf.tetrisai
import move.Move
import piece.Piece
import settings.GameSettings
class Position(board: Board, val piece: Piece, settings: GameSettings) {
val boardWidth = board.width
val boardHeight = board.height
var pieceMove = settings.pieceStartMove(board.width, piece)
val rotationDirection = settings.rotationDirection
def setPiece() = piece.shape(pieceMove.rotation).dots.foreach(dot => board.set(dot + pieceMove, piece.number))
def clearPiece() = piece.shape(pieceMove.rotation).dots.foreach(dot => board.clear(dot + pieceMove))
def rotatePiece() = pieceMove = pieceMove.rotate(rotationDirection, piece.rotationModulus)
def movePiece(move: Move) = pieceMove = move
def isValidMove: Boolean = isValidMove(pieceMove)
def isValidMove(move: Move): Boolean = {
try {
(piece.shape(move.rotation).dots.foldLeft(0) { (sum,dot) => sum + board.get(dot + move) }) == 0
} catch {
case e: IndexOutOfBoundsException => false
}
}
}
I metoden isValidMove på rad 18 låter jag språket självt hålla reda på om biten hamnat utanför brädets kanter genom att låta den fånga IndexOutOfBoundsException. Detta är både en presntandaoptimering och tar samtidigt bort en hel massa boundary-checks som jag annars skulle behövt lägga till.
Klassen Move ser ut så här:
package net.sf.tetrisai.move
import net.sf.tetrisai.piece.Point
object Move {
def apply(y: Int, x: Int, rotation: Int) = new Move(y, x, rotation)
}
class Move(y: Int, x: Int, val rotation: Int) extends Point(x,y) with Ordered[Move] {
def down = new Move(y + 1, x, rotation)
def left = new Move(y, x - 1, rotation)
def right = new Move(y, x + 1, rotation)
def rotate(rotationType: RotationDirection, rotationModulus: Int) = new Move(y, x, (rotation + rotationType.direction) & rotationModulus)
def compare(that: Move): Int = {
if (y != that.y)
y compare that.y
else
if (x != that.x)
x compare that.x
else
rotation compare that.rotation
}
override def equals(that: Any) = that match {
case other: Move => y == other.y && x == other.x && rotation == other.rotation
case _ => false
}
override def toString = "Move(" + y + "," + x + "," + rotation + ")"
}
Det som kan vara värt att nämna här är att jag ärver från klassen Point och att jag gjort klassen sorterbar (mitt test vill ha en förutsägbar ordning på listor av Move). Det är en väldigt elegant syntax när man ska delegera vidare konstruktorns argument till den ärvda klassen (rad 9): extends Point(x,y). En ny instansvariabel position (rad 9) har också tillkommit vilket markeras med nyckelordet val (immutable value).
En annan detalj som kan se konstigt ut för en Javaprogrammerare är syntaxen på rad 17 y compare that.y. Detta tolkas av Scala som y.compare(that.y) och det är en smaksak vilket man väljer!
Vän av ordning undrar nu säkert varför Move ärver från Point. Anledningen är att Move ibland ska hanteras som en Point vilket görs av metoderna setPiece, clearPiece och isValidMove i klassen Position där vi adderar en Point med en Move (rad 13, 14 och 20).
För att stödja olika rotationsriktningar (med- och moturs) i metoden rotate (rad 13 i klassen Move) har jag lagt till den abstrakta klassen RotationDirection:
package net.sf.tetrisai.move
abstract class RotationDirection {
val direction: Int
}
Själva klassen är markerad som abstrakt och attributet direction är här inte satt vilket implicit gör den abstrakt. Mina två implementationer av denna klass måste därför implementera detta värde. ClockwiseRotation:
package net.sf.tetrisai.move
class ClockwiseRotation extends RotationDirection {
val direction = 1
}
AnticlockwiseRotation:package net.sf.tetrisai.move
class AnticlockwiseRotation extends RotationDirection {
val direction = -1
}
Målet med detta projekt är att datorn själv ska kunna spela Tetris. Ett steg på vägen är att leta fram giltiga flytt för en viss position. Klassen ValidMoveCalculator hanterar detta, tillhörande test ser ut så här:
package net.sf.tetrisai.move
import org.junit.Test
import net.sf.tetrisai.settings.DefaultGameSettings
import net.sf.tetrisai.{Position, Board, BaseTest}
import net.sf.tetrisai.piece.PieceS
class ValidMoveCalculatorTest extends BaseTest {
@Test def validMoves() {
val board = Board(Array(
"#-----#",
"#-----#",
"#--X--#",
"#--X--#",
"#######"))
val position = new Position(board, new PieceS, new DefaultGameSettings)
val validMoveCalculator = new ValidMoveCalculator(position)
val moves = validMoveCalculator.validMoves.sorted[Move]
moves should be (List(Move(0,1,0), Move(0,2,0), Move(0,2,1), Move(1,0,0), Move(1,0,1), Move(1,3,1)))
}
}
Implementationen av ValidMoveCalculator:
package net.sf.tetrisai.move
import net.sf.tetrisai.Position
class ValidMoveCalculator(val position: Position) {
var moves: List[Move] = Nil
val visitedMoves = Array.fill(position.boardHeight, position.boardWidth) { 0 }
private def markAsVisited(move: Move) { visitedMoves(move.y)(move.x) |= 1 << move.rotation }
private def isUnvisitedMove(move: Move): Boolean = {
try {
(visitedMoves(move.y)(move.x) & (1 << move.rotation)) == 0
} catch {
case e: IndexOutOfBoundsException => false
}
}
def validMoves = {
moves = List.empty[Move]
calculateValidMoves
moves
}
private def calculateValidMoves {
while (isUnvisitedMove(position.pieceMove) && position.isValidMove) {
val move = position.pieceMove
markAsVisited(move)
if (position.isValidMove(move down)) {
position.movePiece(move down)
calculateValidMoves
} else {
moves = move :: moves
}
position.movePiece(move left)
calculateValidMoves
position.movePiece(move right)
calculateValidMoves
position.movePiece(move)
position.rotatePiece
}
}
}
Hanteringen att ta fram giltiga flytt var i C++ versionen rätt hårt optimerad och
svårbegriplig. Här har jag i stället valt att göra den rekursiv. Då vi med flaggor i visitedMoves markerar vilka sökvägar som redan behandlats bör vi även får bra prestanda. Anledninge till att jag tjatar om bra prestanda är att finjusteringen av algoritmerna som väljer bästa plats för en bit kräver bra prestanda (återstår att implementera). Den låter datorn spela x antal miljoner bitar per parametersättning. Sedan görs en statistiks jämförelse mellan de olika parametersättningarna för att hitta bästa värde för respektive parameter.
