Operating on rows and then on columns of a matrix produces code duplication
-
27-09-2019 - |
Question
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!
Solution
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.
OTHER TIPS
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))
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