3
\$\begingroup\$

I started a Chess in Python for fun. I learned Python in my free time but I've been doing it for a while. The code works but I just want to know if there's anything non-pythonic in my code. The functions calculate all potentially possible moves (ignoring game state).

from itertools import product
from itertools import chain
from functools import wraps
from math import atan2

_angle_to_direction = {90: "up", -90: "down", 180: "right", 0: "left", -135: "right_down", 135: "right_up",
                       45: "left_up", -45: "left_down"}


def move(f):
    @wraps(f)
    def wrapper(*args, **kwargs):
        x, y = args
        moves = f(x, y)
        if (x, y) in moves: moves.remove((x, y))
        return moves

    return wrapper


def check_range(x:tuple) -> bool:
    return x[0] >= 0 and x[1] >= 0 and x[0] < 8 and x[1] < 8


@move
def _knight(x:int, y:int) -> set:
    moves = chain(product([x - 1, x + 1], [y - 2, y + 2]), product([x - 2, x + 2], [y - 1, y + 1]))
    moves = {(x, y) for x, y in moves if check_range((x, y))}
    return moves


@move
def _rook(x:int, y:int) -> set:
    return {(x, i) for i in range(0, 8)}.union({(i, y) for i in range(0, 8)})


@move
def _bishop(x:int, y:int) -> set:
    possible = lambda k: [(x + k, y + k), (x + k, y - k), (x - k, y + k), (x - k, y - k)]
    return {j for i in range(1, 8) for j in possible(i) if check_range(j)}


def direction(start:tuple, end:tuple) -> str:
    delta_a = start[1] - end[1]
    delta_b = start[0] - end[0]
    return _angle_to_direction[int(atan2(delta_a, delta_b) * 180 / 3.14)]
\$\endgroup\$
1
  • \$\begingroup\$ Your check_range() can be simplified to min(x)>=0 and max(x) < 8 or (maybe most pythonic) all(0 <= c < 8 for c in x). Also, why remove (x,y) in the wrapper but check_range() individually in each function? Wouldn't it be more logical to do check_range() also in the wrapper? then you may note that the "vector pieces" rook and bishop (and queen which is the union) work in the same way, you could just give a direction (e.g. (1,1)) and add or subtract as long as check_range(). King and knight go just 1 step in each direction (or opposite), pawn is again slightly different. \$\endgroup\$
    – Max
    Commented Dec 18, 2021 at 3:26

1 Answer 1

2
\$\begingroup\$

The implementation of your move decorator is a little schizophrenic. On the one hand, it carefully allows any and all arguments to be passed to it by declaring itself def wrapper(*args, **kwargs). However it then requires exactly two positional arguments which it then passes on to the wrapped function. Keyword arguments are accepted but ignored and dropped. I would suggest changing this to be all at one end or all at the other end of this spectrum. Given the annotations elsewhere, I would expect you to prefer changing the function to def wrapper(x, y) (or maybe to def wrapper(x, y, **kwargs) but then also passing **kwargs to f).

It might make sense to incorporate check_range into the wrapper as well, simplifying most of the move generators. It might also make sense to skip the decorator and instead call the filter in your move generators: return filter_move((x, y), moves).

As a side note, it would be interesting to see if there are any performance advantages to either the current approach of checking for (x,y) and conditionally removing it or to unconditionally doing a set difference: return f(x, y) - {(x, y)}.

The rest of your code is fairly straightforward, if perhaps a bit too clever. Clever is typically bad for other readers, or for debugging misbehavior. The two things that stand out to me is the name of your lambda possible (which seems more like a diagonal_moves_of_distance or simply diagonal), and the approach used for direction. Given the limited number of possible move deltas, bringing in trigonometry to handle it seems like overkill; I'd almost expect a simple if/else tree instead. As is, it will have problems classifying a knight's move.

\$\endgroup\$
1
  • \$\begingroup\$ thank you sir. i would agree with you being too clever is bad that was my main concern. very detailed answer much appreciated \$\endgroup\$ Commented Dec 11, 2013 at 19:01

Not the answer you're looking for? Browse other questions tagged or ask your own question.