Reversi move class

Question

I redid a Python Reversi move preview routine that is online. I could use comments about using Python better and/or better OOP design since the original did not use classes. There are three main classes:

1. Board for the Reversi board
2. Location to hold an x,y coordinate (0-7,0-7)
3. Move to store a location to put a disc on a board and how many discs would be affected by that move

I separated out Move because I would like to keep a sequence of Moves in memory to play forward or reverse. Board knows about where the discs are in two bitfields, one for each player, and knows how to turn over discs and return if a disc is at a location.

import unittest
import copy
from bitarray import bitarray

INITIAL_BOARD = """
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|B|W|_|_|_|
|_|_|_|W|B|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
"""

def other_player(playerId):
    return 'B' if playerId == 'W' else 'W'

class Location:
    def __init__(self,x=0,y=0):
        self.valid = True
        self.x = x
        self.y = y
        self.offset = self.convertToOffset(self.x,self.y)

    def convertToOffset(self,x,y):
        if self.valid:
            self.valid = (x >= 0 and x <= 7 and y >= 0 and y <= 7)
        return x + y * 8

    def translate(self,dx,dy):
        self.x += dx
        self.y += dy
        self.offset = self.convertToOffset(self.x,self.y)

    def __eq__(self, other):
        if isinstance(other, self.__class__):
            return self.__dict__ == other.__dict__
        else:
            return False

    def __ne__(self, other):
        return not self.__eq__(other)

class Board:
    def __init__(self, board=INITIAL_BOARD):
        self.discs = self.parse_board(board)

    def make_bitstring(self,board,color_current):
        other_color = other_player(color_current)
        def parse_to_bit(square):
            if square == color_current:
                return '1'
            elif square == '_' or square == other_color:
                return '0'
            return ''
        return bitarray(''.join(map(parse_to_bit,board)))

    def parse_board(self,board):
        return (self.make_bitstring(board,'B'),
            self.make_bitstring(board,'W'))

    def show(self):
        print self.discs    

    def at(self, location, player):
        return location.valid and self.discs[int(player=='W')][location.offset]

    def empty(self, location):
        return not location.valid or (not self.at (location,'B') and not self.at (location,'W'))

    def turnoverDisc(self, location):
        offset = location.offset
        self.discs[0][offset], self.discs[1][offset] = self.discs[1][offset], self.discs[0][offset]

    def placeDisc(self, location, player):
        self.discs[int(player=='W')][location.offset] = 1

    def __eq__(self, other):
        if isinstance(other, self.__class__):
            return self.__dict__ == other.__dict__
        else:
            return False

    def __ne__(self, other):
        return not self.__eq__(other)

class Move:
    def __init__(self, board, location, color_player):    
        self.location = location
        self.player = color_player
        self.board = board
        self.discDelta = self.makeDiscDelta()

    def makeDiscDelta(self):
        result = []
        other = other_player(self.player)
        if not self.board.empty(self.location):
            return result
        for dir in [(1,0),(1,-1),(0,-1),(-1,-1),(-1,0),(-1,1),(0,1),(1,1)]:
            next_location = copy.copy(self.location)
            next_location.translate(dir[0],dir[1])
            dirDelta = []
            while self.board.at(next_location,other):
                dirDelta.append(copy.copy(next_location))
                next_location.translate(dir[0],dir[1])
            if self.board.at(next_location,self.player):
                result.extend(dirDelta)
        return result

    def makeBoard(self):
        result = copy.deepcopy(self.board)
        for location in self.discDelta:
            result.turnoverDisc(location)
        result.placeDisc(self.location,self.player)
        return result

class ReversiTest(unittest.TestCase):

    def testMovePreviewWhite(self):
        board = Board()
        location = Location(4,5)
        move = Move(board,location,'W')        
        self.assertEquals([Location(4,4)], move.discDelta)

    def testMovePreviewBlack(self):
        board = Board()
        location = Location(3,2)
        move = Move(board,location,'W')        
        self.assertEquals([Location(3,3)], move.discDelta)

    def testMove(self):
        board = Board()
        expected_board = """
            |_|_|_|_|_|_|_|_|
            |_|_|_|_|_|_|_|_|
            |_|_|_|_|_|_|_|_|
            |_|_|_|B|W|_|_|_|
            |_|_|_|W|W|_|_|_|
            |_|_|_|_|W|_|_|_|
            |_|_|_|_|_|_|_|_|
            |_|_|_|_|_|_|_|_|
            """
        expected = Board(expected_board)
        location = Location(4,5)
        move = Move(board,location,'W') 
        result = move.makeBoard()
        self.assertEquals(expected, result)
        self.assertEquals(board, Board())
        self.assertEquals(location, Location(4,5))

Answer 1

import unittest
import copy
from bitarray import bitarray

INITIAL_BOARD = """
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|B|W|_|_|_|
|_|_|_|W|B|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
|_|_|_|_|_|_|_|_|
"""

def other_player(playerId):
    return 'B' if playerId == 'W' else 'W'

class Location:
    def __init__(self,x=0,y=0):

Put spaces after commas: def __init__(self, x=0, y=0):

        self.valid = True
        self.x = x
        self.y = y
        self.offset = self.convertToOffset(self.x,self.y)

    def convertToOffset(self,x,y):

Call it convert_to_offset, in accordance with the rest of this code (and PEP 8).

        if self.valid:
            self.valid = (x >= 0 and x <= 7 and y >= 0 and y <= 7)

This can be simplified a little using Python's operator chaining: 0 <= x <= 7 and 0 <= y <= 7. But why is this method changing self.valid, when it doesn't change self.x or self.y? Make self.valid into a property, and convert_to_offset into a static method.

        return x + y * 8

    def translate(self,dx,dy):
        self.x += dx
        self.y += dy
        self.offset = self.convertToOffset(self.x,self.y)

    def __eq__(self, other):
        if isinstance(other, self.__class__):
            return self.__dict__ == other.__dict__
        else:
            return False

return NotImplemented instead of False - that way, Python will try (the equivalent of) other.__eq__(self) and only give False if that is also NotImplemented. False means "I know that these two are not equal"; NotImplemented means "I don't know how to compare them".

    def __ne__(self, other):
        return not self.__eq__(other)

First, this should be not self == other, because of the same comparison sequence above. But more importantly, this is the default behavior for self != other, so you might as well omit it.

class Board:
    def __init__(self, board=INITIAL_BOARD):
        self.discs = self.parse_board(board)

    def make_bitstring(self,board,color_current):
        other_color = other_player(color_current)
        def parse_to_bit(square):
            if square == color_current:
                return '1'
            elif square == '_' or square == other_color:
                return '0'
            return ''

Consider moving this function up to module level and passing the colours into it (or, if you really feel like it, use a class for the squares and make this a method on it; but that probably isn't overly worthwhile).

        return bitarray(''.join(map(parse_to_bit,board)))

Consider using a generator expression, especially if you're in Py2 where map builds a list (at least use itertools.imap there) - but I'm quite sure I'm not alone in finding return bitarray(''.join(parse_to_bit(square) for square in board)) more direct and easier to read.

    def parse_board(self,board):
        return (self.make_bitstring(board,'B'),
            self.make_bitstring(board,'W'))

    def show(self):
        print self.discs    

    def at(self, location, player):
        return location.valid and self.discs[int(player=='W')][location.offset]

Consider putting spaces around the ==; but more importantly, there's no need to call int here - bools work fine as sequence indexes (in fact, bool is a subclass of int - you can even add and multiply them).

    def empty(self, location):
        return not location.valid or (not self.at (location,'B') and not self.at (location,'W'))

Do put spaces between arguments (after the commands), don't between the function name (self.at) and the opening bracket.