Automate the Boring Stuff – Chess Dictionary Validator – Optimal Solution

Question

Brief introduction: so the point of the question is that the author doesn't really talk about good or bad code quality and I really wanted some feedback to have a grasp of how the code for others to read should look like. I mean, for the very first solution of the task I would check different parameters manually just to finish it and the function looked really bulky, so I took some time to make it neat.

Task: the dictionary value represent a chess board, for example {'1h': 'bking', '6c': 'wqueen', '2g': 'bbishop', '5h': 'bqueen', '3e': 'wking'}. Write a function named that takes a dictionary argument and returns True or False depending on if the board is valid.

A valid board will have exactly one black king and exactly one white king. Each player can only have at most 16 pieces, at most 8 pawns, and all pieces must be on a valid space from '1a' to '8h'; that is, a piece can’t be on space '9z'. The piece names begin with either a 'w' or 'b' to represent white or black, followed by 'pawn', 'knight', 'bishop', 'rook', 'queen', or 'king'. This function should detect when a bug has resulted in an improper chess board.

My solution:

def chessboardCheck(board):

    piecesDict={'pawn':8,'knight':2,'bishop':2,'rook':2,'queen':1,'king':1} #to count pieces on the board

    if list(board.values()).count('wking')!=1 or list(board.values()).count('bking')!=1: #kings check
        print('The board is invalid: kings requirement not met.')
        return False
    for piece in board.values():
        if piece[0] not in 'wb': #piece color
            print('The board is invalid: improper color reference.')
            return False
        elif piece[1:] not in piecesDict.keys(): #piece type
            print('The board is invalid: improper piece reference.')
            return False
        elif list(board.values()).count(piece) > piecesDict.get(piece[1:]): #pieces count
            print('The board is invalid: improper pieces count.')
            return False
    for cell in board: #axis correctness
        if cell[0] not in '12345678' or cell[1] not in 'abcdefgh':
            print('The board is invalid: improper cell reference.')
            return False

    print('All checks are clear: the board is valid.')
    return True

This is the least lines I could come up with, but if there are more shortcuts to take, please do not hesitate to point! Also I would like to hear opinions on introducing variables: I tried to avoid them wherever I knew how, but maybe it's not always the way to go? Thanks!

Answer 1

As you may know already, your validation rules are both incomplete and too strict. For example, a black bishop must be on a black square, and there can be more-than-expected numbers of the pieces after a pawn is advanced and promoted (usually, but not always, to a queen). I'll ignore those issues and focus on the code.

Get the so-called magic strings and numbers out of the code and into named constants and/or data structures. Here's a draft of some constants that might be handy for your task. As you add more validation checks, you might need to augment or adjust these.

import sys
from collections import namedtuple

BLACK = 'b'
WHITE = 'w'

KING = 'king'
QUEEN = 'queen'
ROOK = 'rook'
BISHOP = 'bishop'
KNIGHT = 'knight'
PAWN = 'pawn'

COLORS = {BLACK, WHITE}
PIECES = {KING, QUEEN, ROOK, BISHOP, KNIGHT, PAWN}

RANKS = set('12345678')
COLUMNS = set('abcdefgh')

VALID_COUNTS = {
    PAWN:   range(0, 9),
    KNIGHT: range(0, 3),
    BISHOP: range(0, 3),
    ROOK:   range(0, 3),
    QUEEN:  range(0, 2),
    KNIGHT: range(1, 2),
}

Set up a simple data structure to facilitate testing and debugging as you write the script. For example, we have the board you gave us and an invalid board I added.

INPUT_BOARDS = {
    'orig': {
        '1h': 'bking',
        '6c': 'wqueen',
        '2g': 'bbishop',
        '5h': 'bqueen',
        '3e': 'wking',
    },
    'bad1': {
        '9h': 'bking',
        '9x': 'wking',
    },
}

The input format for a board is not convenient for validation because it glues together rank-plus-column and color-plus-piece. Do parsing first, validation second. There are various ways to arrange the parsing, but an easy, low-tech way is with a namedtuple -- an immutable container that behaves like a tuple but also allows one to access the attributes via their names. By parsing the input board immediately, you can simplify the rest of the code.

The validation function should take and return data (e.g. the first error or, even better, all errors). It should not print. Print only in the simple, outer-shell of the program (e.g. a main() function), not it its more complex, algorithmic center (where you are doing validation).

ParsedCell = namedtuple('ParsedCell', 'cell color_piece rank column color piece')

def main(args):
    board = parse_input_board(INPUT_BOARDS[args[0]])
    errors = check_board(board)
    if errors:
        for e in errors:
            print(e)
    else:
        print('OK')

def parse_input_board(input_board):
    return tuple(
        ParsedCell(
            cell,
            color_piece,
            cell[0:1],       # A safe technique even if cell is an empty string.
            cell[1:],
            color_piece[0:1],
            color_piece[1:],
        )
        for cell, color_piece in input_board.items()
    )

Now sitting on a more solid foundation, the program's validation code becomes (1) simpler to write and (2) easier to read because it is more declarative or self-documenting.

def check_board(board):
    errors = []
    for pcell in board:
        if pcell.rank not in RANKS:
            msg = emsg('Invalid rank', pcell.cell)
            errors.append(msg)

        if pcell.color not in COLORS:
            msg = emsg('Invalid color', pcell.cell)
            errors.append(msg)

        # Etc.

    return errors

def emsg(msg, item):
    return f'{msg}: {item}'

if __name__ == '__main__':
    main(sys.argv[1:])