I'm working on a method that checks whether pieces are in the way of horizontal, vertical, or diagonal chess moves. (Other types of moves have another method). If the move is from square A to B, the method needs to look at each square between A and B. This is what I came up with as a first draft.
Code:
def clear_path(self):
(x1, y1), (x2, y2) = self.coords
(xmin, xmax), (ymin, ymax) = sorted([x1, x2]), sorted([y1, y2])
if x1 == x2:
for y in range(ymin + 1, ymax):
square = board.notation((x1, y))
if board.piece(square) != None:
print 'Move is not allowed because there is a',
print board.piece(square), 'in the way.'
return False
elif y1 == y2:
for x in range(xmin + 1, xmax):
square = board.notation((x, y1))
if board.piece(square) != None:
print 'Move is not allowed because there is a',
print board.piece(square), 'in the way.'
return False
elif xmax - xmin == ymax - ymin:
if x1 < x2:
x_change = 1
else:
x_change = -1
if y1 < y2:
y_change = 1
else:
y_change = -1
for i in range(xmax - xmin - 1):
x1 += x_change
y1 += y_change
square = board.notation((x1, y1))
if board.piece(square) != None:
print 'Move is not allowed because there is a',
print board.piece(square), 'in the way.'
return False
return True
That looked pretty ugly, so I figured out how to generalize a single approach that would handle all three types of moves and ended up with:
Code:
def clear_path(self):
(x1, y1), (x2, y2) = self.coords
xdist, ydist = (x1 - x2), (y1 - y2)
dist = max(abs(xdist), abs(ydist))
x_change, y_change = xdist/dist , ydist/dist
if x1 == x2 or y1 == y2 or abs(xdist) == abs(ydist):
for d in range(dist - 1):
x1 += x_change
y1 += y_change
square = board.notation((x1, y1))
if board.piece(square) != None:
print 'Move is not allowed because there is a',
print board.piece(square), 'in the way.'
return False
return True
It's a lot shorter, but it still doesn't seem very elegant.
I don't like how I'm basically entering formulas twice. I could write loops, but sometimes it seems like overkill to write a loop to just do one thing twice.
Also, instead of
x_change, y_change = xdist/dist, ydist/dist
I could write something like
x_change, y_change = [i/dist for i in (xdist, ydist)]
But that also seems silly.
Any advice? Should I break the method up into multiple functions? Use more loops?