Skip to main content
Code Review

Return to Revisions

2 of 3
added 142 characters in body
J_H
  • 41.5k
  • 3
  • 38
  • 157

it would be nice to be able to speed it up a bit.

To properly study that, I recommend you scale up from \3ドル ×ばつ 3\$ to \$n ×ばつ n\$.

mutable arg idiom

 if board is None:
 board = [0] * 9

Prefer the usual idiom here:

 board = board or [0] * 9

Or rather, throw that expression into the self.board assignment.

Also, I'm not crazy about repeating the type of self.board in the docstring. It's easy to see that the Optional aspect of the signature has been dealt with by the time we assign the attribute.

enum

 def check_win(self) -> int | None:
 """
 ...
 Returns (int | None):
 Int:
 - 1 if X has won
 - -1 if O has won
 - 0 if game is a draw
 None:
 - None if game is ongoing

Use an enum, please. It could do what disp_dict currently does.

Also, even if we don't use an enum, the returned values seem inconvenient for the caller, as both 0 and None are boolean False, yet they have very different meanings. So caller cannot e.g. say while not board.check_win:

Maybe we want a separate .ongoing property?

speed

Choosing a list-of-lists representation forces you to chase lots of 64-bit object pointers.

Prefer an array, or an ndarray, or even an int bit-vector.

There's only \3ドル^9\$ possible standard game positions (some of which can't be reached from the starting position). You can easily malloc() that many positions, and then a quick dict lookup gives you the desired result.

map

Prefer map() here:

 num_of_moves: int = sum(abs(space) for space in self.board)
 num_of_moves: int = sum(map(abs, self.board))

AssertionError

 try:
 assert ...
 except AssertionError:

No, please don't do that. Use raise ValueError(...), or invent your own MoveError.

When we write assert c, we're saying that a bug-free program would never make c true. Unless you're a pytest maintainer, you should never be catching assertion errors.

Also, I imagine that once the docstring was true. But the current code never re-raises, so it won't report an exception to the caller.

top-level function

This is just odd:

 @classmethod
 def random(cls) -> Self:
 ...
 random_board: Self = cls()

Creating lots of mutable Boards is just fine.

But please have some function up at module-level invoke the Board() ctor to create them. We don't organize Board code by putting it all within the Board class.

J_H
  • 41.5k
  • 3
  • 38
  • 157
default

AltStyle によって変換されたページ (->オリジナル) /