views:

47

answers:

3

I have the following (Python) code to check if there are any rows or columns that contain the same value:

    # Test rows ->  

        # Check each row for a win
        for i in range(self.height):                    # For each row ...

            firstValue = None                           # Initialize first value placeholder

            for j in range(self.width):                 # For each value in the row
                if (j == 0):                                # If it's the first value ...
                    firstValue = b[i][j]                        # Remember it
                else:                                       # Otherwise ...
                    if b[i][j] != firstValue:                   # If this is not the same as the first value ...
                        firstValue = None                           # Reset first value
                        break                                       # Stop checking this row, there's no win here

            if (firstValue != None):                    # If first value has been set
                                                            # First value placeholder now holds the winning player's code
                return firstValue                           # Return it

    # Test columns ->

        # Check each column for a win
        for i in range(self.width):                 # For each column ...

            firstValue = None                           # Initialize first value placeholder

            for j in range(self.height):                # For each value in the column
                if (j == 0):                                # If it's the first value ...
                    firstValue = b[j][i]                        # Remember it
                else:                                       # Otherwise ...
                    if b[j][i] != firstValue:                   # If this is not the same as the first value ...
                        firstValue = None                           # Reset first value
                        break                                       # Stop checking this column, there's no win here

            if (firstValue != None):                    # If first value has been set
                                                            # First value placeholder now holds the winning player's code
                return firstValue                           # Return it

Clearly, there is a lot of code duplication here. How do I refactor this code?

Thanks!

+2  A: 

To check whether all elements in a row are equal, I'd suggest building a python set of the row and then check whether it has only one element. Similarly for the columns.

E.g. like this

def testRowWin(b):
    for row in b:
        if len(set(row)) == 1:
            return True
    return False

def testColWin(b):
    return testRowWin(zip(*b))
jellybean
+1 for `testRowWin(zip(*b))`. I think your first function could be implemented `any(len(set(row)) == 1 for row in b)` if you wanted to be concise like that.
David Zaslavsky
@David: Nice hint, thx. As you mentioned in another comment on this page, backwards compatibility sometimes counts, and `any` has not been introduced until 2.5 :)
jellybean
@jellybean: true, I had completely forgotten that `any` was that new!
David Zaslavsky
+2  A: 

Generally, when you want to refactor, take similar snippets of code and make them into functions. So you could have a function to test all the cells for which one index (either row or column) is the same, and another function that calls that function on all the columns (or rows). Although as Pär pointed out in the comment on your question, it'd be a lot easier to help if you gave some information about what you've tried.

But... another separate (maybe slightly related) matter is that your code doesn't take advantage of Python's functional capabilities. Which is fine, but just so you know, tasks like this where you have to check a bunch of different elements of an array (list, actually) are often much more concise when written functionally. For example, your example could be done like this:

f = lambda x,y: x if x == y else False
# for Python <= 2.4 use this instead:
# f = lambda x,y: x == y and x or False
# test rows
[reduce(f,r) for r in array]
# test columns
reduce(lambda r,s: map(f,r,s), array)

although that's not so helpful if you're trying to understand how the code works.

David Zaslavsky
Why do `x == y and x or False` when you can do `x if x == y else False`. Or better still, write it out as a `def` and avoid packing so much into one line :)
detly
It's an exceptionally short function that does something very simple - those kinds of things can sometimes be clearer as lambdas than as full defined functions. Packing code on to one line is not always a bad thing. And the `x if x == y else False` syntax was only added in Python 2.5; `x == y and x or False` is much older (and more compatible). You're right that the new syntax is probably better if you can use it, though.
David Zaslavsky
+1  A: 
def test_values(self, first, second, HeightWidth):
    for i in range(first):
        firstValue = None
        for j in range(second):
            (firstDimension, secondDimension) = (i, j) if HeightWidth else (j, i)
            if secondDimension == 0:
                firstValue = b[firstDimension][secondDimension]
            else:
                if b[firstDimension][secondDimension] != firstValue:
                    firstValue = None
                    break
    return firstValue

firstValue = test_values(self, self.height, self.width, true)
if firstValue:
    return firstValue

firstValue test_values(self, self.width, self.height, false)
if firstValue:
    return firstValue
utnapistim