Therefore, I would move the definition of numberList
to the main
function. If you don't want a main
function, it should go in the if __name__ == '__main__':
if __name__ == '__main__':
block. If you don't want that, it should at least go after the function definitions. If you don't want that either, ... you're a hopeless case.
Therefore, I would move the definition of numberList
to the main
function. If you don't want a main
function, it should go in the if __name__ == '__main__':
block. If you don't want that, it should at least go after the function definitions. If you don't want that either, ... you're a hopeless case.
Therefore, I would move the definition of numberList
to the main
function. If you don't want a main
function, it should go in the if __name__ == '__main__':
block. If you don't want that, it should at least go after the function definitions. If you don't want that either, ... you're a hopeless case.
Your stringToList
function creates a variable, and does nothing but return it. Why create the variable? Why not just return string.split()
in the first place? I actually don't see any use in creating the function. To me string.split()
is more clear than stringToList(string)
. One could reasonably make a list of each character in a string and call the function stringToList
. The method .split()
, however, is built-in and well documented. Why complicate things?
I'm not sure that it is a good idea to put the grid directly into your code. The more magic values you have, the less reusable your code is. I would suggest that your program should take a filename as an argument and read the grid from there. Of course, you would make sure that the user actually gives a filename, the filename is valid, etc.
You use a list comprehension below, but you don't use one when you convert the list of strings to a list of lists of strings. In fact, I would merge the two:
numberList = [[int(x) for x in line.strip().split()] for line in numberSequence]
If you were reading from a file, you would switch numberSequence
to the name chosen for your file object (numbersFile
if you did something like numbersFile = open(filename)
)
On top of all that, I suggest that you move the definition of numberList
lower down. Usually, code should go like this:
import module
#...
CONSTANT = value
#...
def function():
# ...
def function2():
# ...
#...
def main():
# ...
if __name__ == '__main__':
main()
Therefore, I would move the definition of numberList
to the main
function. If you don't want a main
function, it should go in the if __name__ == '__main__':
block. If you don't want that, it should at least go after the function definitions. If you don't want that either, ... you're a hopeless case.
Your scanning functions are very similar. They are so similar, in fact, that I believe they can be merged:
def scan(numberList, x, y, nums=4):
products = [1] * nums
# Changes we will make to x and y to get the adjacent numbers.
# (0, -1) means x will stay the same, but y will go down by one for
# each adjacent number (down by index, but up visually).
changes = [(0, -1), (0, 1), (1, 0), (-1, 0)]
for i in range(nums):
for p, (xChange, yChange) in enumerate(changes):
try:
products[p] *= numberList[x+(i*xChange)][y+(i*yChange)]
except IndexError:
products[p] = 0
return max(products)
The above has not been tested, but the concept should be fairly clear.
Now that it is all in one function, your largestProduct
definition is fairly simple:
largestProduct = max(
scan(numberList, x, y)
for x, item in enumerate(numberList)
for y in range(len(item))
)
(Again, untested. If you could test it yourself and confirm that it works, I can remove this clutter.)