Python Forum

My code below works. I used a second variable (count) to generate the dictionary. How could I have done this more efficiently?

import random
str = "abcdefg"
print()
print(' This program randomly generates a cipher dictionary from the string',str,end="")
print('.')
print(' Cipher dictionary is:')

def make_cipher_dict(str):
    CIPHER_DICT = {}
    lst = list(str)
    sndlst = list(str)
    random.shuffle(sndlst) #random method changes specified list rather than creating new list
#    print(lst)  DEBUG
#    print(sndlst)  DEBUG
#    quit()  DEBUG
    count = 0
    for a in lst:
        CIPHER_DICT[a] = sndlst[count]
        count = count + 1
    print("",CIPHER_DICT)
    return;

make_cipher_dict(str)

Don't name variables str. There is a built-in function named str, and naming your variable str blocks access to it.

As to efficiency, get rid of count. You're not using it for anything, why are you calculating it? Have you learned zip yet? If so, it can be used to efficiently make a dictionary. The dictionary docs have examples.

Delete line 9 (if you do the rest below)
Delete lines 13-16
Change 17 to

cipher_dict = {a:b for (a,b) in zip(sndlst,str)}

though I agree with not using str as a variable.
Delete 18-20, move the print statement to after line 23 (style issue - your function is called make_cipher_dict, not print it, so would change 21 to return the dictionary and print it from the main program. That makes the function much more re-usable)

I'm not sure a dict comprehension is more efficient than the dict constructor that I was talking about.

Thank you!