The code finds all the combination of alphabet with length 5 and saves it in a list(its ASCII value, minus 97). I want to optimize the code to reduce execution time.

from itertools import product
def permuteKey():

    def pp(chunks):
        return map(''.join, chunks)

    k=pp(product('abcdefghijklmnopqrstuvwxyz', repeat=5))
    key=[]
    for i in range(len(k)):
        temp=k[i]
        temp2=[]
        for j in range(len(temp)):
            temp1=temp[j]
            temp2.append(ord(temp1)-97)
        key.append(temp2)
        print key

permuteKey()
  • It is tough to understand what this code does. You are likely to receive better reviews if you edit your question to explain in words what the code is supposed to do. Could you give examples of expected output with smaller alphabets? – Curt F. Feb 1 '16 at 16:29
  • k=pp(product('ab', repeat=3)) Output is [[0, 0, 0], [0, 0, 1], [0, 1, 0], [0, 1, 1], [1, 0, 0], [1, 0, 1], [1, 1, 0], [1, 1, 1]] – Mrityunjay Vashisth Feb 1 '16 at 16:35
  • That is not words and is not output. – Curt F. Feb 1 '16 at 16:36
  • Dedent print key so that it is not printed every run over the for loop but only after everything has been added to it – Tadhg McDonald-Jensen Feb 1 '16 at 16:40
  • I agree, this post could use some bit more cleaning up. In the meantime, I refactored the code a bit (instead of the post). – Sjoerd Job Postmus Feb 1 '16 at 16:45
up vote 10 down vote accepted

Let me take a stab at this.

from itertools import product
def permuteKey():

    def pp(chunks):
        return map(''.join, chunks)

    k=pp(product('abcdefghijklmnopqrstuvwxyz', repeat=2))
    key=[]
    for i in range(len(k)):
        temp=k[i]
        temp2=[]
        for j in range(len(temp)):
            temp1=temp[j]
            temp2.append(ord(temp1)-97)
        key.append(temp2)
        print key

permuteKey()

Let's start by taking a look at the inner loop:

    temp=k[i]
    temp2=[]
    for j in range(len(temp)):
        temp1=temp[j]
        temp2.append(ord(temp1)-97)

First of all, you're only referring to temp[j] once, and never use j any further. The quick solution would be to iterato over temp instead:

    temp=k[i]
    temp2=[]
    for temp1 in temp:
        temp2.append(ord(temp1)-97)

But now we easily see that temp2 can be written as a list comprehension:

    temp=k[i]
    temp2=[ord(temp1) - 97 for temp1 in temp]

The temp = k[i] can also be removed, leaving

    temp2=[ord(temp1) - 97 for temp1 in k[i]]

Now we end up with

from itertools import product
def permuteKey():

    def pp(chunks):
        return map(''.join, chunks)

    k=pp(product('abcdefghijklmnopqrstuvwxyz', repeat=2))
    key=[]
    for i in range(len(k)):
        temp2=[ord(temp1) - 97 for temp1 in k[i]]
        key.append(temp2)
        print key

permuteKey()

Again, we can apply the trick that k[i] is only used once:

from itertools import product
def permuteKey():

    def pp(chunks):
        return map(''.join, chunks)

    k=pp(product('abcdefghijklmnopqrstuvwxyz', repeat=2))
    key=[]
    for val in k:
        temp2=[ord(temp1) - 97 for temp1 in val]
        key.append(temp2)
        print key

permuteKey()

Another thing I notice, is that you never use the value of one of the items itself, but always ord(c) - 97. Why not just replace abcd...xyz with range(26), and remove the ord(...) - 97 part?

from itertools import product
def permuteKey():

    def pp(chunks):
        return map(''.join, chunks)

    k=product(range(26), repeat=2)
    key=[]
    for val in k:
        temp2=[temp1 for temp1 in val]
        key.append(temp2)
        print key

permuteKey()

But now we don't need pp anymore.

from itertools import product
def permuteKey():

    k=product(range(26), repeat=2)
    key=[]
    for val in k:
        temp2=[temp1 for temp1 in val]
        key.append(temp2)
        print key

permuteKey()

Also, the definition of temp2 is now equal to val.

from itertools import product
def permuteKey():

    k=product(range(26), repeat=2)
    key=[]
    for val in k:
        temp1 = list(val)  # `val` is a tuple, but your code expects a list.
        key.append(temp1)
        print key

permuteKey()

Does it really matter that you want lists instead of tuples? I think not.

from itertools import product
def permuteKey():

    k=product(range(26), repeat=2)
    key=[]
    for val in k:
        key.append(val)
        print key

permuteKey()

Now, I have to wonder: why are we actually printing key at every step? I think you might have made a typo, so I'm going to assume you meant the following: (If not, skip the review from here).

from itertools import product
def permuteKey():

    k=product(range(26), repeat=2)
    key=[]
    for val in k:
        key.append(val)
    print key

permuteKey()

Now, it's bad practise to print inside a function. Better would it be to return the value.

from itertools import product
def permuteKey():

    k=product(range(26), repeat=2)
    key=[]
    for val in k:
        key.append(val)
    return key

print permuteKey()

Now, notice the similarity between k and key. key = list(k).

from itertools import product
def permuteKey():

    k=product(range(26), repeat=2)
    return list(k)

print permuteKey()

But, now you have an intermediate value, you do not need it, and you can remove it.

from itertools import product
def permuteKey():
    return list(product(range(26, repeat=2)))

print permuteKey()

Now, I don't know how often you need permuteKey. If only once, you can write

from itertools import product
print list(product(range(26), repeat=2))
  • range() takes no keyword arguments – Mrityunjay Vashisth Feb 1 '16 at 16:49
  • Ah, a minor typo. Put a ) after the 26, and remove the last one. I fixed the last code-block, so it should work now. – Sjoerd Job Postmus Feb 1 '16 at 16:51
  • 1
    or have a function that takes the length as an argument and returns list(product(range(26),repeat=length)) since you seem to want to repeat for a variable number of times – Tadhg McDonald-Jensen Feb 1 '16 at 17:00
  • @TadhgMcDonald-Jensen: of course, if you need this multiple times, with or without variable lengths: yes. – Sjoerd Job Postmus Feb 1 '16 at 17:05
  • Thank you @SjoerdJobPostmus. My coding is very messy. Have to work hard on it. Thanks for the solution. helps a lot – Mrityunjay Vashisth Feb 1 '16 at 17:08

Your Answer

 
discard

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.