patternpythonMinor
Calculating potential Chess moves
Viewed 0 times
potentialcalculatingchessmoves
Problem
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] 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)]Solution
The implementation of your
It might make sense to incorporate
As a side note, it would be interesting to see if there are any performance advantages to either the current approach of checking for
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
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.Context
StackExchange Code Review Q#37089, answer score: 2
Revisions (0)
No revisions yet.