English 中文(简体)
Python: does it make sense to refactor this check into its own method?
原标题:

I m still learning python. I just wrote this method to determine if a player has won a game of tic-tac-toe yet, given a board state like: [[ o , x , x ],[ x , o , - ],[ x , o , o ]]

def hasWon(board):
  players = [ x ,  o ]
  for player in players:
    for row in board:
      if row.count(player) == 3:
        return player
    top, mid, low = board
    for i in range(3):
      if [ top[i],mid[i],low[i] ].count(player) == 3:
        return player
    if [top[0],mid[1],low[2]].count(player) == 3:
        return player
    if [top[2],mid[1],low[0]].count(player) == 3:
        return player
  return None

It occurred to me that I check lists of 3 chars several times and could refactor the checking to its own method like so:

def check(list, player):
  if list.count(player) == 3:
    return player

...but then realized that all that really does is change lines like:

 if [ top[i],mid[i],low[i] ].count(player) == 3:
    return player

to:

  if check( [top[i],mid[i],low[i]], player ):
    return player

...which frankly doesn t seem like much of an improvement. Do you see a better way to refactor this? Or in general a more Pythonic option? I d love to hear it!

最佳回答

I might use

def check(somelist, player):
  return somelist.count(player) == 3

Edit: as @Andrew suggested in a comment (tx @Andrew!), you can do even better, e.g.:

def check(somelist, player):
  return somelist.count(player) == len(somelist)

without hardcoding the 3 -- which also suggests another nice alternative:

def check(somelist, player):
  return all(x==player for x in somelist)

which very directly reads "all items in the list equal player". The general point is that by refactoring to a separate method you can then play with that method s implementation -- now of course here the code is very simple so the advantage is similarly modest, but it s an excellent point to keep in mind as you move to more complicated code.

As you ve noticed you only need a bool anyway, so this allows a much simpler approach -- just return the bool expression rather than doing an if on it. It s important to never use a built-in name like list for your own identifiers -- an "attractive nuisance" of the language...;-).

By which I mean, Python uses for its built-ins lots of nice, attractive names like list, bool, sum, and so on, so it s easy to find yourself accidentally using one of those names for a variable of your own, and nothing bad seems to happen... until the time you need to turn, say, a tuple into a list, use the obviously best solution, x = list(thetuple)... and end up spending our trying to understand and solve the errors that come because you ve used list to mean anything else than the built-in type of that name.

So, just get into the habit of not using those nice built-in names for purposes other than indicating the respective builtins, and you ll save yourself much future aggravation!-)

Back to your code, you might consider the conciseness afforded by not unpacking board (a hard decision, since your code is quite readable... but may look a bit verbose):

for i in range(3):
  if check([row[i] for row in board], player):
    return player
if check([row[i] for i, row in enumerate(board)], player):
    return player
if check([row[2-i] for i, row in enumerate(board)], player):
    return player

In the end I think I d stick with your choice -- more readable and just marginally more verbose, if at all -- but it s nice to be aware of the alternatives, I think -- here, list comprehensions and enumerate to generate the lists to be checked as an alternative to "manually coding out" the three possibilities.

问题回答

You could use a better name instead of check that doesn t tell much. The rule of thumb is: if you can think of a good name for a peace of code then it might be beneficial to move it into separate function even if it is just one line of code. allsame might be one of alternatives here.

def winner(board):
    main_diag = [row[i] for i, row in enumerate(board)]
    aux_diag = [row[len(board) - i - 1] for i, row in enumerate(board)]   
    for triple in board + zip(*board) + [main_diag, aux_diag]: 
        if allsame(triple):         
           return triple[0]

def allsame(lst):    
    return all(x == lst[0] for x in lst)

Simply make a custom iterator over board.

def get_lines(board):
  nums = range(3)
  for i in nums: 
    yield (board[i][j] for j in nums) #cols
  for j in nums: 
    yield (board[i][j] for i in nums) #rows
  yield (board[i][i] for i in nums) #diag 
  yield (board[i][2-i] for i in nums) #diag /

def get_winner(board): #a bit too indented
  for line in get_lines(board): #more expensive, so go through it only once
    for player in  x ,  o :
      if line == player, player, player: #other way to check victory condition
        return player
  return None

Obviously these really should be methods of a board class :)

Personally I think your best bet for readability is to bubble out functions to give you the rows(), columns(), and diags() of the board, as lists of lists. Then you can iterate through these and check uniformly. You can even then define allTriples(), which appends the output of rows(), columns(), and diags(), so that you can do your checking in a single concise loop. I d probably also make the board an object, so that these functions could become object methods.

And now for something completely different:

Represent the board by a list of nine elements. Each element can be -1 (X), 1 (O), or 0 (empty):

WIN_LINES = (
    (0, 1, 2),
    (3, 4, 5),
    (6, 7, 8),
    (0, 3, 6),
    (1, 4, 7),
    (2, 5, 8),
    (2, 4, 6),
    (0, 4, 8),
    )

def test_for_win(board):
    for line in WIN_LINES:
        total = sum(board[point] for point in line)
        if abs(total) == 3:
            return total // 3
    return None

Refinement:

WIN_LINES = (
    0, 1, 2,
    3, 4, 5,
    6, 7, 8,
    0, 3, 6,
    1, 4, 7,
    2, 5, 8,
    2, 4, 6,
    0, 4, 8,
    )

def test_for_win(board):
    wpos = 0
    for _unused in xrange(8):
        total  = board[WIN_LINES[wpos]]; wpos += 1
        total += board[WIN_LINES[wpos]]; wpos += 1
        total += board[WIN_LINES[wpos]]; wpos += 1
        if total ==  3: return  1
        if total == -3: return -1
    return None

Just an idea

def hasWon(board):
  players = [ x ,  o ]
  for player in players:
    top, mid, low = board
    game = board + [[ top[i],mid[i],low[i]] for i in range(3)] + [top[0],mid[1],low[2]] +[top[2],mid[1],low[0]]
    if 3 in [l.count(player) for l in game] :
      return player
  return None

Your solution is fine - correct, readable and understandable.

Still, if you d want to optimize for speed, I d use a 1-dimensional array of digits, not strings, and try to look up each number as few times as possible. There will certainly be an extremely awkward-looking solution in which you check every field only once. I don t want to construct this now. :) Things like that might matter if you wanted to implement an AI playing against you while exploring the entire search tree of possible moves. An efficient win/loss check would be necessary there.





相关问题
Can Django models use MySQL functions?

Is there a way to force Django models to pass a field to a MySQL function every time the model data is read or loaded? To clarify what I mean in SQL, I want the Django model to produce something like ...

An enterprise scheduler for python (like quartz)

I am looking for an enterprise tasks scheduler for python, like quartz is for Java. Requirements: Persistent: if the process restarts or the machine restarts, then all the jobs must stay there and ...

How to remove unique, then duplicate dictionaries in a list?

Given the following list that contains some duplicate and some unique dictionaries, what is the best method to remove unique dictionaries first, then reduce the duplicate dictionaries to single ...

What is suggested seed value to use with random.seed()?

Simple enough question: I m using python random module to generate random integers. I want to know what is the suggested value to use with the random.seed() function? Currently I am letting this ...

How can I make the PyDev editor selectively ignore errors?

I m using PyDev under Eclipse to write some Jython code. I ve got numerous instances where I need to do something like this: import com.work.project.component.client.Interface.ISubInterface as ...

How do I profile `paster serve` s startup time?

Python s paster serve app.ini is taking longer than I would like to be ready for the first request. I know how to profile requests with middleware, but how do I profile the initialization time? I ...

Pragmatically adding give-aways/freebies to an online store

Our business currently has an online store and recently we ve been offering free specials to our customers. Right now, we simply display the special and give the buyer a notice stating we will add the ...

Converting Dictionary to List? [duplicate]

I m trying to convert a Python dictionary into a Python list, in order to perform some calculations. #My dictionary dict = {} dict[ Capital ]="London" dict[ Food ]="Fish&Chips" dict[ 2012 ]="...

热门标签