"Largest product in a series" Python code

Question

After converting a string into an array, I really don't know why this isn't working... Can you explain what's my error, please?

Find the thirteen adjacent digits in the 1000-digit number that have the greatest product. What is the value of this product?

seq = '7316717653133062491922511967442657474235534919493496983520312774506326239578318016984801869478851843858615607891129494954595017379583319528532088055111254069874715852386305071569329096329522744304355766896648950445244523161731856403098711121722383113622298934233803081353362766142828064444866452387493035890729629049156044077239071381051585930796086670172427121883998797908792274921901699720888093776657273330010533678812202354218097512545405947522435258490771167055601360483958644670632441572215539753697817977846174064955149290862569321978468622482839722413756570560574902614079729686524145351004748216637048440319989000889524345065854122758866688116427171479924442928230863465674813919123162824586178664583591245665294765456828489128831426076900422421902267105562632111110937054421750694165896040807198403850962455444362981230987879927244284909188845801561660979191338754992005240636899125607176060588611646710940507754100225698315520005593572972571636269561882670428252483600823257530420752963450'
new = []
for n in seq:
    new.append(int(n))

max_prod = 0
for i in range(0, len(new)-13):
    product = 1
    for j in range(i, i+13):
        product *= j
    if product > max_prod:
        max_prod = product
print(max_prod)

Answer 1

In your loop, you multiply the product by j: product *= j, but you want to multiply it by element in new with the index j, therefore just change the line product *= j to product *= new[j]

Corrected program:

seq = '7316717653133062491922511967442657474235534919493496983520312774506326239578318016984801869478851843858615607891129494954595017379583319528532088055111254069874715852386305071569329096329522744304355766896648950445244523161731856403098711121722383113622298934233803081353362766142828064444866452387493035890729629049156044077239071381051585930796086670172427121883998797908792274921901699720888093776657273330010533678812202354218097512545405947522435258490771167055601360483958644670632441572215539753697817977846174064955149290862569321978468622482839722413756570560574902614079729686524145351004748216637048440319989000889524345065854122758866688116427171479924442928230863465674813919123162824586178664583591245665294765456828489128831426076900422421902267105562632111110937054421750694165896040807198403850962455444362981230987879927244284909188845801561660979191338754992005240636899125607176060588611646710940507754100225698315520005593572972571636269561882670428252483600823257530420752963450'
new = []
for n in seq:
    new.append(int(n))

max_prod = 0
for i in range(0, len(new)-12):
    product = 1
    for j in range(i, i+13):
        product *= new[j]
    if product > max_prod:
        max_prod = product
print(max_prod)

Answer 2

The answer by @karan-elangovan shows what is wrong with your code. You can improve your code using list comprehensions and slices.

For example

new = []
for n in seq:
    new.append(int(n))

becomes

new = [int(n) for n in seq]

and

product = 1
for j in range(i, i+13):
    product *= new[j]

can become

product = 1
for n in new[i:i+13]:
    product *= n

Answer 3

Here are a few more optimizations you may want to consider. The direct approach requires 988 x 12 = 11,856 multiplications. We can reduce that to 527 multiplications and 239 divisions:

from functools import reduce
from operator import mul

seq = '7316717653133062491922511967442657474235534919493496983520312774506326239578318016984801869478851843858615607891129494954595017379583319528532088055111254069874715852386305071569329096329522744304355766896648950445244523161731856403098711121722383113622298934233803081353362766142828064444866452387493035890729629049156044077239071381051585930796086670172427121883998797908792274921901699720888093776657273330010533678812202354218097512545405947522435258490771167055601360483958644670632441572215539753697817977846174064955149290862569321978468622482839722413756570560574902614079729686524145351004748216637048440319989000889524345065854122758866688116427171479924442928230863465674813919123162824586178664583591245665294765456828489128831426076900422421902267105562632111110937054421750694165896040807198403850962455444362981230987879927244284909188845801561660979191338754992005240636899125607176060588611646710940507754100225698315520005593572972571636269561882670428252483600823257530420752963450'

# we need only consider stretches with no zero in it:
bits = seq.split('0')

# comparing neighboring stretches
# 
# bit[i] * bit[i+1] * bit[i+2] * ... * bit[i+12]
#          bit]i+1] * bit[i+2] * ... * bit[i+12] * bit[i+13]
#
# we see that they have 12 factors in common, so we can get the second product
# by dividing the first by bit[i] and multiplying by bit[i+13]
#
def max13(bit):
    if len(bit) < 13:
        return 0
    else:
        # next line does the same as @DanielGee's list comprehension
        # map applies its first arg (a function) to each element of its
        # second arg (a sequence) to create a new sequences
        # in Python2 the output used to be a list, in Python3 it's an iterator
        # so we need to cast to tuple or list
        bit = tuple(map(int, bit))
        mx = current = reduce(mul, bit[:13])
        for hire, fire in zip(bit[13:], bit[:-13]):
            # important: use integer division '//'
            current //= fire
            current *= hire
            if current > mx:
                mx = current
        return mx

# finally, we just need to take the max over all bits
result = max(map(max13, bits))