Simple encoder by a beginner

Question

I decided to make a quick little encoder in Python as a little challenge for myself. I decided to submit my program as I am very interested in improving my programming skills and learning better ways to code. My program simply asks for a string and for a key. The key could be +3, +4, etc. The key will add the number inputted to the number corresponding to each letter, then return the new letters.

dic = {"A": 0, "B": 1, "C": 2, "D": 3, "E": 4, "F": 5, "G": 6, "H": 7, "I": 8, "J": 9, "K": 10, "L": 11, "M": 12, "N": 13, "O": 14, "P": 15, "Q": 16, "R": 17, "S": 18, "T": 19, "U": 20, "V": 21, "W": 22, "X": 23, "Y": 24, "Z": 25, " ": 26}
dic2 = {}
for x,v in dic.items():
 dic2[v] = x
program = True
while program:
 list = []
 encodeNums = []
 encodeStr = []
 task = input("Would you like to encode or decode?: ").lower()
 if task == "encode":
 string = input("What would you like to encode?: ").upper()
 key = input("What key would you like to use?: ")
 for x in string:
 list.append(dic[x])
 for x in list:
 encodeNums.append(eval("{}{}".format(x,key)) % 27)
 for x in encodeNums:
 encodeStr.append(dic2[x])
 print("".join(encodeStr))

Answer 1

I'd highly suggest learning comprehensions. The simplest is a list comprehension. This allows you to change say:

list = []
for x in string:
 list.append(dic[x])

To:

list = [dic[x] for x in string]

However, you can get better memory usage by using a generator comprehension, this works in roughly the same way as a list, however you can't index it. Which for this code is fine. To do this replace the square brackets with curved ones:

gen = (dic[x] for x in string)

You can also use a dictionary comprehension to simplify your creation of dic2:

dic2 = {v: k for k, v in dic.items()}

---

Purge all memory of eval, it leads to poor code and has severe security vulnerabilities. Instead cast the users input to an int. This can simplify decoding too. However you have to make sure the user does actually enter a number.

I don't like list = [] being outside the if statement. There's not much of a need for this. However it's fixed when using comprehensions.

I also think you should use while True rather than while program:. As you never change program.

---

Merging this all together gets you:

dic = {"A": 0, "B": 1, "C": 2, "D": 3, "E": 4, "F": 5, "G": 6, "H": 7, "I": 8, "J": 9, "K": 10, "L": 11, "M": 12, "N": 13, "O": 14, "P": 15, "Q": 16, "R": 17, "S": 18, "T": 19, "U": 20, "V": 21, "W": 22, "X": 23, "Y": 24, "Z": 25, " ": 26}
dic2 = {v: k for k, v in dic.items()}
while True:
 task = input("Would you like to encode or decode?: ").lower()
 if task == "encode":
 string = input("What would you like to encode?: ").upper()
 while True:
 try:
 key = int(input("What key would you like to use?: "))
 break
 except ValueError:
 pass
 list = (dic[x] for x in string)
 encodeNums = ((x + key) % 27 for x in list)
 encodeStr = (dic2[x] for x in encodeNums)
 print("".join(encodeStr))

Going forward, I'd change your code to encode and decode with the same code. This is as the Caesar cipher is a symmetric cypher, so simply using key = -key allows you to decode the encoding.

Allowing:

dic = {"A": 0, "B": 1, "C": 2, "D": 3, "E": 4, "F": 5, "G": 6, "H": 7, "I": 8, "J": 9, "K": 10, "L": 11, "M": 12, "N": 13, "O": 14, "P": 15, "Q": 16, "R": 17, "S": 18, "T": 19, "U": 20, "V": 21, "W": 22, "X": 23, "Y": 24, "Z": 25, " ": 26}
dic2 = {v: k for k, v in dic.items()}
while True:
 string = input("What would you like cypher?: ").upper()
 task = input("Would you like to encode or decode?: ").lower()
 while True:
 try:
 key = int(input("What key would you like to use?: "))
 break
 except ValueError:
 pass
 if task == "decode":
 key = -key
 list = (dic[x] for x in string)
 encodeNums = ((x + key) % 27 for x in list)
 encodeStr = (dic2[x] for x in encodeNums)
 print("".join(encodeStr))

Answer 2

First, your dic may be created more comfortably, e.g. from the string of all uppercase letters

alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'

(which is iterable) and

range(26) # Generator of numbers 0, 1, ..., 25

First create pairs of their corresponding items using the zip() function:

zip(alphabet, range(26))

and then use the dict() constructor with that as its as its argument, so you will obtain:

dic = dict(zip(alphabet, range(26)))

---

Second, you use modulo 27 and number of English letters is only 26.

---

Third, the name list for a list is not very descriptive for its content.

---

Fourth, the

eval("{}{}".format(x,key)) % 27

in your statement

encodeNums.append(eval("{}{}".format(x,key)) % 27)

is not only a horrible construction (it is best to avoid the eval() function as much as possible) but the result is not what you wanted. You wanted simply

(x + key ) % 26 # NOT 27

• Enrique: thanks for your advice! I decided to do modulo 27 because I also included a space " " in my dictionary. I did this to make it easier to decode the string and also to add more variety to my alphabet. Thank you for your advice :)
• MarianD: Oh, my fault - I didn't scroll you code enough to the right.