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

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).

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

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]]