我以编写此代码为例,在发布该代码作为代码之前,我想确保它尽可能好例如。
很少有假设:
该代码应仅处理小块动作,而不应管理整个游戏。
该代码不应处理诸如传球,en回,或典当促销。
如果有其他威胁威胁,则代码不应强迫国王移动。
from abc import ABC, abstractmethod
class Color:
BLACK = 0
WHITE = 1
def enemy_of(color):
if color == Color.BLACK:
return Color.WHITE
return Color.BLACK
class Board:
BOARD_SIZE = (8, 8)
def __init__(self):
self.reset()
def get_square(self, row, col):
if not self.is_valid_square((row, col)):
return None
return self.board[row][col]
def set_square(self, row, col, piece):
self.board[row][col] = piece
def is_valid_square(self, square):
return (
square[0] in range(self.BOARD_SIZE[0])
and square[1] in range(self.BOARD_SIZE[1])
)
def is_empty_square(self, square):
return self.get_square(*square) is None
def _generate_first_row(self, color):
row_by_color = {Color.BLACK: 0, Color.WHITE: self.BOARD_SIZE[0] - 1}
row = row_by_color[color]
order = (Rook, Knight, Bishop, Queen, King, Bishop, Knight, Rook)
params = {'color': color, 'row': row}
return [order[i](col=i, **params) for i in range(self.BOARD_SIZE[0])]
def _generate_pawns_row(self, color):
row_by_color = {Color.BLACK: 1, Color.WHITE: self.BOARD_SIZE[0] - 2}
row = row_by_color[color]
params = {'color': color, 'row': row}
return [Pawn(col=i, **params) for i in range(self.BOARD_SIZE[0])]
def get_pieces(self, color=None):
for row in self.board:
for col in row:
if col is not None and (color is None or col.color == color):
yield col
def get_possible_moves(self, color, with_king=False):
"""Return all player's possible moves."""
pieces = self.get_pieces(color=color)
if not with_king:
pieces = [p for p in pieces if not isinstance(p, King)]
for piece in pieces:
for move in piece.get_valid_moves(self):
yield move
def reset(self):
self.board = [
self._generate_first_row(Color.BLACK),
self._generate_pawns_row(Color.BLACK),
[None] * self.BOARD_SIZE[0],
[None] * self.BOARD_SIZE[0],
[None] * self.BOARD_SIZE[0],
[None] * self.BOARD_SIZE[0],
self._generate_pawns_row(Color.WHITE),
self._generate_first_row(Color.WHITE),
]
def move(self, source, destination):
piece = self.get_square(*source)
return piece.move(board=self, destination=destination)
def __str__(self):
printable = ""
for row in self.board:
for col in row:
if col is None:
printable = printable + " ▭ "
else:
printable = printable + f" {col} "
printable = printable + '\n'
return printable
class Piece(ABC):
def __init__(self, color, row, col, **kwargs):
super().__init__(**kwargs)
self.color = color
self.row = row
self.col = col
def is_possible_target(self, board, target):
is_target_valid = board.is_valid_square(target)
is_empty_square = board.is_empty_square(target)
is_hitting_enemy = self.is_enemy(board.get_square(*target))
return is_target_valid and (is_empty_square or is_hitting_enemy)
@abstractmethod
def get_valid_moves(self, board):
pass
def get_position(self):
return self.row, self.col
def is_enemy(self, piece):
if piece is None:
return False
return piece.color == Color.enemy_of(self.color)
def move(self, board, destination):
if not self.is_possible_target(board, destination):
return False
if destination not in self.get_valid_moves(board):
return False
board.set_square(*self.get_position(), None)
board.set_square(*destination, self)
self.row, self.col = destination
return True
@abstractmethod
def __str__(self):
pass
class WalksDiagonallyMixin:
def __init__(self, **kwargs):
super().__init__(**kwargs)
if not hasattr(self, 'directions'):
self.directions = set()
self.directions.update({
(-1, -1), (1, -1),
(-1, 1), (1, 1),
})
class WalksStraightMixin:
def __init__(self, **kwargs):
super().__init__(**kwargs)
if not hasattr(self, 'directions'):
self.directions = set()
self.directions.update({
(0, -1),
(-1, 0), (1, 0),
(0, 1),
})
class WalksMultipleStepsMixin:
def get_valid_moves(self, board):
for row_change, col_change in self.directions:
steps = 1
stop_searching_in_this_direction = False
while not stop_searching_in_this_direction:
new_row = self.row + row_change * steps
new_col = self.col + col_change * steps
target = (new_row, new_col)
is_valid_target = self.is_possible_target(board, target)
if is_valid_target:
yield target
steps = steps + 1
is_hit_enemy = self.is_enemy(board.get_square(*target))
if not is_valid_target or (is_valid_target and is_hit_enemy):
stop_searching_in_this_direction = True
class Pawn(Piece):
DIRECTION_BY_COLOR = {Color.BLACK: 1, Color.WHITE: -1}
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.moved = False
self.forward = self.DIRECTION_BY_COLOR[self.color]
def _get_regular_walk(self):
src_row, src_col = self.get_position()
return (src_row + self.forward, src_col)
def _get_double_walk(self):
src_row, src_col = self.get_position()
return (src_row + self.forward * 2, src_col)
def _get_diagonal_walks(self):
src_row, src_col = self.get_position()
return (
(src_row + self.forward, src_col + 1),
(src_row + self.forward, src_col - 1),
)
def is_possible_target(self, board, target):
is_valid_move = board.is_valid_square(target)
is_step_forward = (
board.is_empty_square(target)
and target == self._get_regular_walk()
)
is_valid_double_step_forward = (
board.is_empty_square(target)
and not self.moved
and target == self._get_double_walk()
and self.is_possible_target(board, self._get_regular_walk())
)
is_hitting_enemy = (
self.is_enemy(board.get_square(*target))
and target in self._get_diagonal_walks()
)
return is_valid_move and (
is_step_forward or is_valid_double_step_forward or is_hitting_enemy
)
def move(self, **kwargs):
is_success = super().move(**kwargs)
self.moved = True
return is_success
def get_valid_moves(self, board):
targets = (
self._get_regular_walk(),
self._get_double_walk(),
*self._get_diagonal_walks(),
)
for target in targets:
if self.is_possible_target(board, target):
yield target
def __str__(self):
if self.color == Color.WHITE:
return '♙'
return '♟'
class Bishop(WalksDiagonallyMixin, WalksMultipleStepsMixin, Piece):
def __str__(self):
if self.color == Color.WHITE:
return '♗'
return '♝'
class Rook(WalksStraightMixin, WalksMultipleStepsMixin, Piece):
def __str__(self):
if self.color == Color.WHITE:
return '♖'
return '♜'
class Queen(
WalksStraightMixin, WalksDiagonallyMixin, WalksMultipleStepsMixin, Piece,
):
def __str__(self):
if self.color == Color.WHITE:
return '♕'
return '♛'
class Knight(Piece):
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.directions = [
(-2, 1), (-1, 2), (1, 2), (2, 1), # Upper part
(-2, -1), (-1, -2), (1, -2), (2, -1), # Lower part
]
def get_valid_moves(self, board):
for row_change, col_change in self.directions:
row, col = self.get_position()
target = (row + row_change, col + col_change)
if self.is_possible_target(board, target):
yield target
def __str__(self):
if self.color == Color.WHITE:
return '♘'
return '♞'
class King(WalksStraightMixin, WalksDiagonallyMixin, Piece):
def _get_threatened_squares(self, board):
enemy = Color.enemy_of(self.color)
enemy_moves = list(board.get_possible_moves(enemy, with_king=False))
enemy_pieces = board.get_pieces(color=enemy)
king = next(p for p in enemy_pieces if isinstance(p, King))
for move in king.get_squares_threatens(board):
yield move
for move in enemy_moves:
yield move
def is_possible_target(self, board, target):
is_regular_valid = super().is_possible_target(board, target)
threatened_squares = self._get_threatened_squares(board)
return is_regular_valid and target not in threatened_squares
def get_valid_moves(self, board):
for add_row, add_col in self.directions:
target = (add_row + self.row, add_col + self.col)
if self.is_possible_target(board, target):
yield target
def get_squares_threatens(self, board):
for direction in self.directions:
row, col = self.get_position()
row = row + direction[0]
col = col + direction[1]
if board.is_valid_square((row, col)):
yield (row, col)
def __str__(self):
if self.color == Color.WHITE:
return '♔'
return '♚'
我知道我可以改善的地方,但是由于我目前的学生所学,我还是保持现状:
我可以使用
yield from
代替for x in y: yield x
。颜色可以继承
enum.Enum
并使用enum.auto()
用于类变量。我可以引发异常,而不是返回
True
或False
。#1 楼
我将在文件顶部添加一条注释,以指示您所使用的相关软件的版本。快速声明“使用Python 3.6测试(通过Anaconda安装)”或类似的注释,可以确保每个人都在同一页面上。由于这是作为教学示例,因此专注于最小化当前代码。我认为这是一个合理的假设,即更多的代码为潜在的混乱提供了更多的空间。
class Board:
BOARD_SIZE = (8, 8)
您是否会拥有一块非正方形的面板?这可以是一个简单的int吗?更改此设置会使代码的总数量减少不小的数量。函数),设置器并没有真正增加代码。 getter有点气味,因为getter返回None是意外的,并且提供的使用getter的代码都不会检查None。我会同时删除这两个函数。
def get_square(self, row, col):
if not self.is_valid_square((row, col)):
return None
return self.board[row][col]
def set_square(self, row, col, piece):
self.board[row][col] = piece
如果使用不当,此功能将很不方便调试。例如,如果输入参数'square'为空,则会出现错误。
def is_valid_square(self, square):
return (
square[0] in range(self.BOARD_SIZE[0])
and square[1] in range(self.BOARD_SIZE[1])
)
严格来说,参数square可以是任意大小,但我们希望它有两个元素。我可以通过拆包,断言或更改函数签名来使该假设明确化。
名称为_generate_back_row。我认为这是一个更清晰的名称。快速的维基百科搜索告诉我,要使用的确切术语是排名第一或排名倒数,但这可能还不够为人所知。
此功能正在进行中。我认为可以利用一点只有两种颜色的事实来简化一点。字典查找和从字典中扩展kwarg都是过大的(但是,两者都是很好的教学方法,我将其留在_generate_pawn中)。该代码可能类似于
>>> board.is_valid_square([])
Traceback (most recent call last):
...
square[0] in range(self.BOARD_SIZE[0])
IndexError: list index out of range
def is_valid_square(self, row, col):
return row in range(self.BOARD_SIZE) and col in range(self.BOARD_SIZE)
我认为变量col应该命名为square。 color = None是什么意思?获得两种颜色的碎片?该功能未在代码中的任何地方使用。我认为应该简化此功能,删除默认参数。我认为代码看起来更合理
def _generate_first_row(self, color):
row_by_color = {Color.BLACK: 0, Color.WHITE: self.BOARD_SIZE[0] - 1}
row = row_by_color[color]
order = (Rook, Knight, Bishop, Queen, King, Bishop, Knight, Rook)
params = {'color': color, 'row': row}
return [order[i](col=i, **params) for i in range(self.BOARD_SIZE[0])]
def _generate_back_row(self, color):
row = 0 if color == Color.BLACK else self.BOARD_SIZE - 1
order = (Rook, Knight, Bishop, Queen, King, Bishop, Knight, Rook)
return [
order[i](col=i, row=row, color=color)
for i in range(self.BOARD_SIZE[0])
]
注释有点混乱。我们在说哪个球员? with_king是什么意思?我本来希望所有可能的动作默认都包括国王的动作。我建议使用类似下面的内容,它会翻转默认值,包括可能的国王动作,但要强调该功能可以选择不包括它们。好的功能。它调用的函数名称使逻辑清晰易懂。我会考虑将定义更改为
return is_target_valid and not is_hitting_self
,因为这对于计算机来说工作量较小,但是总体来说看起来确实不错。 def get_pieces(self, color=None):
for row in self.board:
for col in row:
if col is not None and (color is None or col.color == color):
yield col
我将对该函数的逻辑进行一些小的更改。它具有相当大的复杂性(3个缩进,一个收益和一个直接影响下一个if语句的if语句),因此为其提供更多的空格,并反转一些布尔值可能会使它更整洁,更重要的是,易于解析。
要更改的第一件事是将内部逻辑移至其自身的功能。这有两个好处,它使代码更易于解析,并且允许内部逻辑在需要时停止,而不是显式跟踪循环条件。
def get_color_pieces(self, color):
for row in self.board:
for square in row:
if square is not None and square.color == color:
yield square
is_hit_enemy仅在其中设置第一个if语句,在此之前甚至不存在。我将尝试将逻辑保持在一个位置(并将名称更改为has_hit_enemy,因为那样会更准确)。为此,请反转条件以使其成为保护子句。
def get_possible_moves(self, color, with_king=False):
"""Return all player's possible moves."""
由于仅用于停止循环,因此便于删除stop_searching_in_this_direction。因为我们可以返回,所以它变得没有必要了。我要说掉它
def get_possible_moves(self, color, exclude_king=False):
"""Return all possible moves using the pieces of the specified color."""
实际上,由于每个片段都知道自己的行和列,所以为什么我们仍然需要self.get_position()?
def is_possible_target(self, board, target):
is_target_valid = board.is_valid_square(target)
is_empty_square = board.is_empty_square(target)
is_hitting_enemy = self.is_enemy(board.get_square(*target))
return is_target_valid and (is_empty_square or is_hitting_enemy)
def is_enemy(self, piece):
if piece is None:
return False
return piece.color == Color.enemy_of(self.color)
逻辑看起来不错,但是很难在代码中找到它。我看到的is_valid_square越多,我就越不喜欢该名称。考虑其他可以让您知道函数检查内容的名称,例如is_within_bounds或is_inside。我还注意到,每个返回布尔值的函数都以is_为前缀,几乎达到了病理程度。还有其他一些更适合的前缀,例如has,can,will或只是省略了前缀。有了guard子句,并更改前缀使其更有意义,代码可能看起来像
def get_valid_moves(self, board):
for row_change, col_change in self.directions:
steps = 1
stop_searching_in_this_direction = False
while not stop_searching_in_this_direction:
new_row = self.row + row_change * steps
new_col = self.col + col_change * steps
target = (new_row, new_col)
is_valid_target = self.is_possible_target(board, target)
if is_valid_target:
yield target
steps = steps + 1
is_hit_enemy = self.is_enemy(board.get_square(*target))
if not is_valid_target or (is_valid_target and is_hit_enemy):
stop_searching_in_this_direction = True
def get_valid_moves(self, board):
for row_change, col_change in self.directions:
for move in moves_in_a_direction(self, row_change, col_change):
yield move
def moves_in_a_direction(self, row_change, col_change):
steps = 1
stop_searching_in_this_direction = False
while not stop_searching_in_this_direction:
new_row = self.row + row_change * steps
new_col = self.col + col_change * steps
target = (new_row, new_col)
is_valid_target = self.is_possible_target(board, target)
if is_valid_target:
yield target
steps = steps + 1
is_hit_enemy = self.is_enemy(board.get_square(*target))
if not is_valid_target or (is_valid_target and is_hit_enemy):
stop_searching_in_this_direction = True
可以,但是不太清楚。重新排列线路并将国王重命名为敌人国王会改善代码。
if not is_valid_target:
return
yield target
steps += 1
has_hit_enemy = ...
...
但这提出了一个问题:“为什么敌方国王受到不同待遇?”当然,这只是敌人的另一部分,有一系列可能的动作,每个动作都会威胁这位国王?如果此处有注释,请添加注释以解释该注释。
评论
\ $ \ begingroup \ $
你太棒了。感谢您的不懈努力和良好的精神。我敢肯定,我会采纳您的许多建议。
\ $ \ endgroup \ $
–无限
20年7月6日在1:50
\ $ \ begingroup \ $
你会不会有一个非正方形的木板?美国邮政编码会超过5位吗?是否需要指定日期的世纪?这里的人名或地名会超过任意数字吗?客户是否会希望空数字字段为空而不是零?社会保障号码会合法地重复使用吗?
\ $ \ endgroup \ $
– radarbob
20年7月10日在17:54
\ $ \ begingroup \ $
@radarbob邮政编码可以为九位数字,称为ZIP + 4,用于指定街道地址或邮政信箱。
\ $ \ endgroup \ $
–琳妮
20年7月11日在20:59
#2 楼
由于@ spyr03的广泛(且很棒)答案未包含此内容,因此这里有一些小注释。您应该在每个类,方法和函数中包含一个docstring
,以详细说明其功能以及其参数和返回值。尽管您的代码是自我记录的文档,但这为他们编写自己的代码开创了先例。如果您一贯做到这一点(并要求他们这样做),那么有些人可能会学会做到这一点。至少在最后,不要忘记教他们编写(好的)测试的方法。尤其是这些作品很容易成为测试的候选者。它们具有复杂的非平凡行为,您在更改某些内容时可能会弄混,因此对它们进行全面的测试覆盖将非常有帮助。
在实践中,当我参加
Pawn
课程时,我有些惊讶。首先,为运动类型定义这些漂亮的mixin。但是,Pawn
类不会使用它们中的任何一个!我知道典当可能是您要定义的第一部分,而且在这种情况下使用mixins有点困难,但我会考虑从实际使用的部分开始是否更好mixins之一。或稍后在实际需要它们时对其进行定义。#3 楼
当我回想起学生时代时,对我而言,理解代码的最关键点始终是切入点。以我的经验,需要从整体上理解一个代码概念。未经培训的人员将用于逐步思考和逐步评估。我不会理解该代码,因为它描述了游戏并且不玩游戏。我了解,该代码不是要播放的。但是一个清晰标记的start()
函数可以初始化电路板并执行一些示例动作,从而使学生能够看到并可视化代码的组合方式以及其实际功能,这将大有帮助。至少对我有帮助。评论
\ $ \ begingroup \ $
“是的,然后……”如果您采用的代码“初始化电路板并执行一些示例动作”,然后在末尾添加一些断言— ta-da,您将进行首次单元测试!单元测试可以帮助读者理解代码的行为及其预期用途,并且可以同时充当测试。强烈推荐。
\ $ \ endgroup \ $
– Quuxplusone
20年7月7日在3:55
#4 楼
我在spyr03的出色评论中没有提到一件事:我认为使用mixin类为get_valid_moves
,Rook
和Bishop
使用mixin类实现Queen
例程的100%不必要地不一致(从而造成混淆) King
的代码之一(并打开另一半的代码)。如果要编写
class Queen(
WalksStraightMixin, WalksDiagonallyMixin, WalksMultipleStepsMixin, Piece,
): #######################
def __str__(self):
if self.color == Color.WHITE:
return '♕'
return '♛'
,那么您还应该编写
class King(
WalksStraightMixin, WalksDiagonallyMixin, WalksSingleStepMixin, Piece,
): ####################
令
WalksStraightMixin
和WalksDiagonallyMixin
将值设置到self.directions
中,然后由King
自己读取这些值,造成了混乱。这是mixins和King
的实现之间的紧密耦合依赖;考虑一下如果要将directions
重命名为possibleDirections
或类似的东西,必须在代码中更改多少个地方。太复杂。我们可以通过手动为每个类分别实现get_valid_moves
来“保持简单”:class Piece:
straight_directions = [...]
diagonal_directions = [...]
all_directions = straight_directions + diagonal_directions
def get_single_step_moves_impl(directions): ...
def get_multistep_moves_impl(directions): ...
class Rook(Piece):
def get_valid_moves(self):
return self.get_multistep_moves_impl(Piece.straight_directions)
class Queen(Piece):
def get_valid_moves(self):
return self.get_multistep_moves_impl(Piece.all_directions)
class King(Piece):
def get_valid_moves(self):
return self.get_single_step_moves_impl(Piece.all_directions)
这里,而不是从mixins继承(可能会影响整个类的行为),我们限制了“不同”效果尽量缩小。
Queen
对get_multistep_moves_impl
的使用与King
对get_single_step_moves_impl
的使用之间的区别显然仅限于get_valid_moves
。显然,Queen
和King
的行为除了get_valid_moves
的行为外没有其他区别(无论如何也没有如上所述)。这种可能的局限性使读者更容易推理代码。#5 楼
自从我问这个问题已经有一段时间了。我根据您的建议改进了代码,并将其作为练习提供给我的学生。这是巨大的成功。我将详细介绍一些改进,作为对我问题的后续回答。感谢您的出色回答。多么伟大的社区:)
我已经为所有函数和类添加了文档字符串。参数,一个用于行,一个用于列,而不是单个元组。它还使用两个存储两个布尔值的变量来简化调试功能。
老:
is_valid_square
新增:
def is_valid_square(self, row, column):
"""Return True if square in board bounds, False otherwise."""
row_exists = row in range(self.BOARD_SIZE[0])
column_exists = column in range(self.BOARD_SIZE[1])
return row_exists and column_exists
def is_valid_square(self, square):
return (
square[0] in range(self.BOARD_SIZE[0])
and square[1] in range(self.BOARD_SIZE[1])
)
的名称更改为generate_first_row
。generate_back_row
现在包含属性piece
和moved
。我发现direction
可能会更容易使用此数据来管理典当/城堡,而且我认为这可能会使将来的传奇作品受益。在实例启动时将moved
创建为空集将使其更易于管理和继承。direction
已更新为@ spyr03的建议:旧的:
is_enemy
def is_enemy(self, piece):
if piece is None:
return False
return piece.color == Color.enemy_of(self.color)
我添加了
def is_enemy(self, piece):
"""Return if the piece belongs to the opponent."""
if piece is None:
return False
return piece.color != self.color
来简化具有其他击打方式(例如典当)的方式: get_squares_threatens
我更改了类的顺序以突出显示mixins的用法。现在在
def get_squares_threatens(self, board):
"""Get all the squares which this piece threatens.
This is usually just where the piece can go, but sometimes
the piece threat squares which are different from the squares
it can travel to.
"""
for move in self.get_valid_moves(board):
yield move
之前定义了Rook
和Queen
。我已将Pawn
添加到get_squares_threatens
父类中。 Piece
使用该类来检查它是否可以移动到特定的广场。它极大地简化了King
方法。我可能还没有提到其他一些改进,因此在此附加了更新的代码:)
_get_threatened_squares
评论
\ $ \ begingroup \ $
如果黑人国王在B3上,则不允许白人国王在C3上继续。
\ $ \ endgroup \ $
–罗兰·伊利格(Roland Illig)
20 Dec 5'在8:28
\ $ \ begingroup \ $
很好。这是国际象棋的工作方式:)
\ $ \ endgroup \ $
–无限
20 Dec 5'14:23
\ $ \ begingroup \ $
但是您在get_squares_threatens的文档中说,白国王可能会继续使用C3。即使国王“看起来”受到主教的保护,也不允许国王去那里。
\ $ \ endgroup \ $
–罗兰·伊利格(Roland Illig)
20 Dec 5 '21:12
评论
请不要更新您问题中的代码以合并答案的反馈,否则会违反“代码审查”的“问题+答案”样式。这不是一个论坛,您应该在其中保留问题的最新版本。收到答案后,请查看您可能会做什么和可能不会做什么。随意发布带有修改后代码的新问题,自我回答指出您的改进之处以及为什么或以其他方式共享代码。随时添加链接。但请不要将其添加到问题本身中。谢谢!关于最后一点,无论如何返回True或False可能总比引发错误要好