Determine the height of a Tetris game's board after a sequence of moves

Question

I have written a program that will determine the height of a Tetris board after a sequence of moves are made. These inputs are in the form of a comma-delimited list, and look like <piece><position>. List of pieces:

• I - this is a 1x4 piece lying on its side
• Q - this is a 2x2 square piece
• T - this is a T-shaped piece
• Z - this is a left-facing 2x2 offset
• S - this is a right-facing 2x2 offset
• L - this is a right-facing L
• J - this is a left-facing L

Image (source) of the pieces. Pieces are always in the same orientation as below.

I've diagrammed them out below as well. Rotation is not in scope for this problem (e.g. a vertical I is out of scope).

I - xxxx
Q - xx
    xx
T - xxx
     x
Z - xx
     xx
S -  xx
    xx
L - x
    x
    xx
J -  x
     x
    xx

Positions are 0-indexed, and represent a location from the left side of the board (the board is 10-wide).

Example 1:

Input: I0,Q4

Output: 2

Board:

bbbbQQbbbb
IIIIQQbbbb

(b represents a blank space, and the blank lines above this are left out)

Example 2

Input: Q0,Q2,Q4,Q6,Q8

Output: 0

Board (intentionally left blank):

Explanation: Using normal Tetris rules, a row is removed whenever every block in a row is filled. This sequence would place 5 square cubes evenly spaced along the bottom, which then removes those two rows.

class Tetris:
    def __init__(self):
        self.board =[]
        self.pieces = {
            'I' : [[1,1,1,1]],

            'Q' : [[1,1],
                   [1,1]],

            'T': [[1,1,1],
                  [0,1,0]],

            'Z':[[1,1,0],
                 [0,1,1]],

            'S':[[0,1,1],
                 [1,1,0]],

            'L':[[1,0],
                 [1,0],
                 [1,1]],

            'J':[[0,1],
                 [0,1],
                 [1,1]]}

    def newRow(self):
        return [0 for _ in range(10)]

    def doesThePieceFit(self,row,pieceName,pos):
        #checks to see if a piece fits on the row at given position
        #check bottom to the top
        piece = self.pieces[pieceName]
        for i in range(len(piece)):
           pieceRow = piece[-1*(1+i)]
           if i+row == len(self.board): return True
           boardRow = self.board[i+row]
           for j in range(len(pieceRow)):
               if pieceRow[j] and boardRow[pos+j]: return False
        return True

    def removeFullRows(self,startRow,numRows):
        #removes full rows from the board
        #only checks rows between startRow and startRow+numRows
        fullRows = [i+startRow
                    for i in range(numRows)
                    if all(self.board[i+startRow])]
        for fullRow  in sorted(fullRows,reverse=True):
            del self.board[fullRow]

    def addPieceAt(self,row,pieceName,pos):
        #Adds piece at this row.
        piece = self.pieces[pieceName]
        for i in range(len(piece)):
           pieceRow = piece[-1*(1+i)]
           if i+row == len(self.board):
               self.board+=self.newRow(),
           boardRow = self.board[i+row]
           for j in range(len(pieceRow)):
               if pieceRow[j]:
                   boardRow[pos+j] = pieceRow[j]
        self.removeFullRows(row,len(piece))

    def addPiece(self,pieceName,pos):
        #1.find the first row where piece is blocked
        #2.Add the piece at the row above it
        blockedByRow = None
        for row in range(len(self.board)-1,-1,-1):
            if not self.doesThePieceFit(row,pieceName,pos):
                blockedByRow = row
                break

        targetRow = 0 if  blockedByRow == None else blockedByRow+1
        self.addPieceAt(targetRow,pieceName,pos)

    def addPieces(self,pieces):
        for piece in pieces.split(','):
            self.addPiece(piece[0],int(piece[1]))
        return len(self.board)

Answer 1

The first thing I did was use Black to reformat the code - yours is pretty good, but there are some minor style complaints I had (generally around the lack of whitespace in a few places). Additionally, PEP8 defines the naming conventions in python - generally, prefer_this notThis.

Lastly, all of your methods should have docstrings. I haven't added this b/c it isn't as pertinent to the code review, but it is good practice in general.

From there, I thought about your actual approach. At a high level you:

• Create a new instance of the object
• Pass it a string, parse the string, and process each token
• Attempt to fit pieces
• Clear full rows

None of that is inherently bad, but I think it can be tightened up a bit.

User Input

Right now you don't have any validation of the user inputs - we're being very trusting that the values that are provided will be usable. We probably want to do this validation

Additionally, I don't think that the Tetris class should be responsible for handling the comma-delimited string - it should just take a piece and a position, and something else should be responsible for taking the input and translating it into arguments. If you're feeling friendly, a @classmethod might be appropriate. Lastly, I think this class method should return the board, not the height, so I added a new height property to the class. I ended up with something like this:

pieces = {
    "I": ((True, True, True, True)),
    "Q": ((True, True), (True, True)),
    "T": ((True, True, True), (False, True, False)),
    "Z": ((True, True, False), (False, True, True)),
    "S": ((False, True, True), (True, True, False)),
    "L": ((True, False), (True, False), (True, True)),
    "J": ((False, True), (False, True), (True, True)),
}

@classmethod
def add_pieces(cls, user_input):
    board = Tetris()
    for piece in user_input.split(","):
        if len(piece) > 2:
            raise ValueError(f"Piece {piece} is malformed")
        piece_id = piece[0]
        drop_position = piece[1]
        if not Tetris.is_valid_piece(piece_id):
            raise ValueError(f"Piece {piece_id} is not a valid Tetris piece")
        if not Tetris.is_valid_drop_location(drop_position):
            raise IndexError(
                f"Drop location {drop_position} is not a valid board location"
            )
        board.add_piece(piece_id, drop_position)
    return board

@classmethod
def is_valid_piece(cls, piece_id):
    return piece_id in cls.pieces

@classmethod
def is_valid_drop_location(drop_position):
    try:
        int(drop_position)
    except ValueError:
        return False

    return drop_position >= 0 and drop_position < 10

@property
def height(self):
    return self.board.length

You'll also notice that I moved Tetris.pieces into a class attribute instead of an instance attribute - this is because it should be the same everywhere. I also changed 0/1 to True/False because it is a binary value (I think an enum is probably best to be explicit, e.g. boardState.FULL and boardState.EMPTY). Lastly, I changed from nested lists to nested tuples - this is because tuples are immutable, and you never need to change the shape definition.

OOP

I wonder if it is worthwhile making a separate class to represent the pieces, and then you can do something like TetrisPiece.fitsAtLocation(board, location). I haven't fully thought about what this would look like or if it is actually better, but it might be a nice way to encapsulate that functionality.

This would also be a convenient way to extend this to handle rotations as well, as you would just do TetrisPiece.rotate(Direction.LEFT) and handle it all under the hood.

If you want to extend this to a full game, then instead of just having a "drop position" you also need a relative location on the board, handling T-spins, etc. The more complicated this gets, the more I think a separate class is going to improve readability.

General nitpicks

• doesThePieceFit seems really weird - I get how it works, but you should definitely introduce some constants to replace the magic method, and maybe consider if there is a better way to model the data.
  • In particular, perhaps we should store the block state for a different shape in reverse order (e.g. bottom-to-top instead of top-to-bottom)?
• removeFullRows creates a list, then sorts it - I think you can probably come up with a different approach for this
• addPieceAt has the same magic as doesThePieceFit - is there a way that we can either combine their functionality, or use a common helper method?
• addPiece I think you can use for-else to handle this a bit more elegantly than using the ternary, but my mood on the for-else swings every time I use it

Answer 2

Your code is good but it is not intuitive to interface with grafically.

I can print the board but it comes out reversed and as zeros and ones and I got to do:

>>> t = Tetris()
>>> print(t.board)

But you can use the special method repr to make it print nicely automagically (whenever the user asks print(t))

In Python 3 you can just add this at the end of your class:

class Tetris:
    # other code

    def __repr__(self):
        return '\n'.join(reversed([''.join("■" if elem else '□' for elem in line) for line in t.board]))

And now you have an intuitive and graphically nice pretty print:

t = Tetris()
for piece, pos in ( ('L',1), ('Z', 2), ('S', 3), ('I',5)):
    t.addPiece(piece, pos)
    print(t)
    print("\n"*5)

Outputs:

□■□□□□□□□□
□■□□□□□□□□
□■■□□□□□□□







□■□□□□□□□□
□■■■□□□□□□
□■■■■□□□□□






□□□□■■□□□□
□■□■■□□□□□
□■■■□□□□□□
□■■■■□□□□□






□□□□□■■■■□
□□□□■■□□□□
□■□■■□□□□□
□■■■□□□□□□
□■■■■□□□□□

In Python 2 you might have to use ASCII characters but this allows for easy developing and testing and is necessary in case you want to turn this into a game.

(It looks way nicer in Python IDLE than in this site).

Answer 3

1. Use Booleans instead of Integers: The code uses integers to check if a cell is occupied or not. Example: Replace I = [1,1,1,1] with I=[True,True,True,True]

2. Mark internal functions with underscores: By python convention, any function that is not meant to be invoked from outside the class is usually marked with underscores. Example: Replace def addPiece(...) with def _addPiece_(...).

3. Use meaningful variable names: Use meaningful names for variables (including iterator variables) Don't use arbitrary names like i or j. Looking at the variable names, it isn't clear whether doesThePieceFit validates columns at all

4. Handling invalid input: You can return an error value (throw a python error or return integer value -1) for invalid inputs. (Such as I9 on a size 10 board)

---

Also, if you can change the input format, you can make some minor changes to make this code more useful. You can change the constructor to __init__(self,size) instead of fixing the size to 10. Also, you can change the input format from string "Q0,Q2" to list [["Q",0],["Q",2]]