HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Project Euler Problem 11: largest product of 4 adjacent numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
adjacentproblemnumbersproductlargestprojecteuler

Problem

I am a fairly beginner-level programmer, and I have been working through the Project Euler Puzzles. I am currently on Problem #11, which asks to find the largest product of four consecutive numbers (horizontally, vertically, or diagonally) in a grid.

I would like someone to look at my code and give me any tips, pointers, and better techniques and styles I could use. I think I have the grasp of functional programming fairly well, and am try to get better at OOP, so I am also wondering if there is any way to code this in an OOP manner.

```
# coding: utf-8
def stringToList(string):
numberList = string.split()

return numberList

numberSequence = ["08 02 22 97 38 15 00 40 00 75 04 05 07 78 52 12 50 77 91 08",
"49 49 99 40 17 81 18 57 60 87 17 40 98 43 69 48 04 56 62 00",
"81 49 31 73 55 79 14 29 93 71 40 67 53 88 30 03 49 13 36 65",
"52 70 95 23 04 60 11 42 69 24 68 56 01 32 56 71 37 02 36 91",
"22 31 16 71 51 67 63 89 41 92 36 54 22 40 40 28 66 33 13 80",
"24 47 32 60 99 03 45 02 44 75 33 53 78 36 84 20 35 17 12 50",
"32 98 81 28 64 23 67 10 26 38 40 67 59 54 70 66 18 38 64 70",
"67 26 20 68 02 62 12 20 95 63 94 39 63 08 40 91 66 49 94 21",
"24 55 58 05 66 73 99 26 97 17 78 78 96 83 14 88 34 89 63 72",
"21 36 23 09 75 00 76 44 20 45 35 14 00 61 33 97 34 31 33 95",
"78 17 53 28 22 75 31 67 15 94 03 80 04 62 16 14 09 53 56 92",
"16 39 05 42 96 35 31 47 55 58 88 24 00 17 54 24 36 29 85 57",
"86 56 00 48 35 71 89 07 05 44 44 37 44 60 21 58 51 54 17 58",
"19 80 81 68 05 94 47 69 28 7

Solution

Here's what I see:

  • When doing Project Euler exercises I like to drop a link and a copy of the subject in the module docstring: it's nice when you share it and read it later.



  • Your numberSequence big-array is breaking the 80th column, which I personally don't like.



  • You do not need a stringToList function, just use .split(). It may look more readable as you're naming your intention, which is really nice, and you named it well, which is really nice. BUT we all know .split() and it's no surprise. So when we encounter stringToList we have to go read the code, which is slower and harder than just reading .split(). So naming intention is a nice idea, but don't over-do it, it costs you the time to write a single-line function and cost us the time to find it and read it for something commonly known (str.split(), please).



  • Indentation in your numberSequence is messed up, I don't know why but remember that in Python you should not use tabs to indent your code, only blocks of 4 spaces (see the PEP8). It's nice when you share code with 4 spaces that it appears everywhere equals (and by not using tabs we're sure nobody mixes tabs and spaces ...).



  • You're calling each scan* method twice if the product is greater than the previous, you're not nice with the CPU (think like you're doing it yourself)



  • Your numberList variable means two different things in two different scopes: global and locals, which should probably be avoided to avoid bugs (like thinking you're hitting the global one, but modifying the local one ?)



  • PEP8 tells us to put two newlines between functions.



  • Your four scan methods are essentially the same, only the signs change. Maybe they can be combined?



  • for i in numberSequence: numberList.append(stringToList(i)) should probably be more readable as a list comprehension.



I would go for something more like:

from contextlib import suppress

numberSequence = [
    "08 02 22 97 38 15 00 40 00 75 04 05 07 78 52 12 50 77 91 08",
    "49 49 99 40 17 81 18 57 60 87 17 40 98 43 69 48 04 56 62 00",
    "81 49 31 73 55 79 14 29 93 71 40 67 53 88 30 03 49 13 36 65",
    "52 70 95 23 04 60 11 42 69 24 68 56 01 32 56 71 37 02 36 91",
    "22 31 16 71 51 67 63 89 41 92 36 54 22 40 40 28 66 33 13 80",
    "24 47 32 60 99 03 45 02 44 75 33 53 78 36 84 20 35 17 12 50",
    "32 98 81 28 64 23 67 10 26 38 40 67 59 54 70 66 18 38 64 70",
    "67 26 20 68 02 62 12 20 95 63 94 39 63 08 40 91 66 49 94 21",
    "24 55 58 05 66 73 99 26 97 17 78 78 96 83 14 88 34 89 63 72",
    "21 36 23 09 75 00 76 44 20 45 35 14 00 61 33 97 34 31 33 95",
    "78 17 53 28 22 75 31 67 15 94 03 80 04 62 16 14 09 53 56 92",
    "16 39 05 42 96 35 31 47 55 58 88 24 00 17 54 24 36 29 85 57",
    "86 56 00 48 35 71 89 07 05 44 44 37 44 60 21 58 51 54 17 58",
    "19 80 81 68 05 94 47 69 28 73 92 13 86 52 17 77 04 89 55 40",
    "04 52 08 83 97 35 99 16 07 97 57 32 16 26 26 79 33 27 98 66",
    "88 36 68 87 57 62 20 72 03 46 33 67 46 55 12 32 63 93 53 69",
    "04 42 16 73 38 25 39 11 24 94 72 18 08 46 29 32 40 62 76 36",
    "20 69 36 41 72 30 23 88 34 62 99 69 82 67 59 85 74 04 36 16",
    "20 73 35 29 78 31 90 01 74 31 49 71 48 86 81 16 23 57 05 54",
    "01 70 54 71 83 51 54 69 16 92 33 48 61 43 52 01 89 19 67 48"]

numberList = [line.split() for line in numberSequence]
numberList = [[int(x) for x in y] for y in numberList]

def multiply(iterable):
    result = 1
    for number in iterable:
        result *= number
    return result

def scan(x, y):
    with suppress(IndexError):
        yield multiply(numberList[x][y+i] for i in range(4))
    with suppress(IndexError):
        yield multiply(numberList[x+i][y] for i in range(4))
    with suppress(IndexError):
        yield multiply(numberList[x+i][y-i] for i in range(4))
    with suppress(IndexError):
        yield multiply(numberList[x+i][y+i] for i in range(4))

def main():
    all_products = []
    for x in range(len(numberList)):
        for y in range(len(numberList[0])):
            for product in scan(x, y):
                all_products.append(product)
    largestProduct = max(all_products)

    print(largestProduct)
    assert largestProduct == 70600674

main()

Code Snippets

from contextlib import suppress


numberSequence = [
    "08 02 22 97 38 15 00 40 00 75 04 05 07 78 52 12 50 77 91 08",
    "49 49 99 40 17 81 18 57 60 87 17 40 98 43 69 48 04 56 62 00",
    "81 49 31 73 55 79 14 29 93 71 40 67 53 88 30 03 49 13 36 65",
    "52 70 95 23 04 60 11 42 69 24 68 56 01 32 56 71 37 02 36 91",
    "22 31 16 71 51 67 63 89 41 92 36 54 22 40 40 28 66 33 13 80",
    "24 47 32 60 99 03 45 02 44 75 33 53 78 36 84 20 35 17 12 50",
    "32 98 81 28 64 23 67 10 26 38 40 67 59 54 70 66 18 38 64 70",
    "67 26 20 68 02 62 12 20 95 63 94 39 63 08 40 91 66 49 94 21",
    "24 55 58 05 66 73 99 26 97 17 78 78 96 83 14 88 34 89 63 72",
    "21 36 23 09 75 00 76 44 20 45 35 14 00 61 33 97 34 31 33 95",
    "78 17 53 28 22 75 31 67 15 94 03 80 04 62 16 14 09 53 56 92",
    "16 39 05 42 96 35 31 47 55 58 88 24 00 17 54 24 36 29 85 57",
    "86 56 00 48 35 71 89 07 05 44 44 37 44 60 21 58 51 54 17 58",
    "19 80 81 68 05 94 47 69 28 73 92 13 86 52 17 77 04 89 55 40",
    "04 52 08 83 97 35 99 16 07 97 57 32 16 26 26 79 33 27 98 66",
    "88 36 68 87 57 62 20 72 03 46 33 67 46 55 12 32 63 93 53 69",
    "04 42 16 73 38 25 39 11 24 94 72 18 08 46 29 32 40 62 76 36",
    "20 69 36 41 72 30 23 88 34 62 99 69 82 67 59 85 74 04 36 16",
    "20 73 35 29 78 31 90 01 74 31 49 71 48 86 81 16 23 57 05 54",
    "01 70 54 71 83 51 54 69 16 92 33 48 61 43 52 01 89 19 67 48"]


numberList = [line.split() for line in numberSequence]
numberList = [[int(x) for x in y] for y in numberList]


def multiply(iterable):
    result = 1
    for number in iterable:
        result *= number
    return result


def scan(x, y):
    with suppress(IndexError):
        yield multiply(numberList[x][y+i] for i in range(4))
    with suppress(IndexError):
        yield multiply(numberList[x+i][y] for i in range(4))
    with suppress(IndexError):
        yield multiply(numberList[x+i][y-i] for i in range(4))
    with suppress(IndexError):
        yield multiply(numberList[x+i][y+i] for i in range(4))


def main():
    all_products = []
    for x in range(len(numberList)):
        for y in range(len(numberList[0])):
            for product in scan(x, y):
                all_products.append(product)
    largestProduct = max(all_products)

    print(largestProduct)
    assert largestProduct == 70600674

main()

Context

StackExchange Code Review Q#130088, answer score: 2

Revisions (0)

No revisions yet.