Successful Encoder and Decoder for messages

I recently wrote a code for a encoder and decoder that works off of a key and the only way to decode the message is with this program and the case-sensitive key. I have yet to incorporate punctuation but that will come soon. I am currently designing a GUI using the Tkinter module. I would love any feedback.

```
from random import seed, shuffle
#Encoder Function
def Encoder(user_input,SEED):
user_input = user_input.lower()
letter = ["a","b","c","d","e","f","g","h","i","j","k",'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z']
Letter_code = {"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}
code = ["a","b","c","d","e","f","g","h","i","j","k",'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z',]
n = []
seed(SEED)
shuffle(code)
for letter in user_input:
for let in letter:
if letter != " ":
if letter == let:
first = Letter_code[let]
n.append(code[first])
else:
n.append("~")
return ''.join(n)

#Decoder Function
def Decoder(user_input,SEED):
user_input = user_input.lower
key_list = ["a","b","c","d","e","f","g","h","i","j","k",'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z']
final = ["a","b","c","d","e","f","g","h","i","j","k",'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z']
seed(SEED)
shuffle(key_list)
key_code = {}
z = 0
n = []
for key in key_list:
key_code[key] = z
z += 1
for let in user_input:
if let != "~":
for Ke in key_list:
if let == Ke:
a = key_code[Ke]
n.append(final[a])
else:
n.append(" ")
return ''.join(n)

#Prompt
encode_decode = raw_input("Would you like to encode or decode a message?(encode/decode)")
encode_

Concept

Interesting idea to use the pseudo-random number generator seed as an encryption key. Note, however, that there's no guarantee that the PRNG implementation will be consistent from release to release of Python, or that it will act identically across platforms. The same goes for the shuffle() function.

As you probably know, you should never try to implement your own cryptography for any meaningful usage, and this substitution cipher is trivially crackable by letter frequency analysis. I'll treat this as nothing more than a fun programming exercise.

Correctness

In the decoder, the call to lower() is missing parentheses. The return statement is incorrectly indented.

In the encoder, the letter iteration dummy variable shadows the letter array defined earlier. In effect, the code becomes:

def Encoder(user_input,SEED):
    user_input = user_input.lower()
    letter = ["a","b","c","d","e","f","g","h","i","j","k",'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z']
    Letter_code = {"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}
    code = ["a","b","c","d","e","f","g","h","i","j","k",'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z',]
    n = []
    seed(SEED)
    shuffle(code)
    for letter in user_input:    # Note: This letter variable shadows the previous letter
        let = letter             # Originally… for let in letter:
        if letter != " ":
            if True:             # Originally… if letter == let:
                first = Letter_code[let]
                n.append(code[first])
        else:
            n.append("~")
    return ''.join(n)

That happens to mostly work! The only thing that breaks was that you had intended to filter out unencodable characters, and now it will raise a KeyError if user_input contains anything other than letters or spaces.

If you dig deeper for a root cause of your bug, you can blame it on a proliferation of variables, causing confusion. See if you can eliminate some variables, and name the remaining ones better.

Efficiency

In the encoder, your if letter != " " should be put between the outer and inner for-loops. The decoder does it right.

Nested for-loops are a sign of poor efficiency. Lookups should be done using a dictionary. A dictionary would be more efficient (constant-time access rather than a scan through 26 list items), and it would look cleaner in the code. (If you were doing serious cryptography, you would need all operations to be constant-time, to thwart attacks based on analyzing timing.)

Style

• Naming conventions:

• Encoder(user_input, SEED) → encode(user_input, seed) because it's a function, not a class. You want a verb, not a noun. SEED is not a constant.

• Decoder(user_input, SEED) → decode(user_input, seed) for the same reasons.

• letter → LETTER because it's a constant.

• Letter_code → LETTER_CODE because it's a constant.

• n → encoded or n → decoded because n has the connotation of being a number.

• Prefer list comprehensions to loops: If you ever want to transform one list into another list, use a list comprehension instead of a for-loop if it's at all possible. It's one assignment statement, and it forces you to express yourself using higher-level functions rather than step-by-step procedural code.

• Work smarter, not harder: Save yourself some error-prone typing — let the computer generate your lists of letters and their corresponding codes.

• Symmetry: The decoder is just the inverse of the encoder, so there's no reason why it should be any more complex.

-
If-else: Whenever you have an if-else construct, prefer to put the branch with the shorter body first to reduce mental workload. For example, write the decoder this way:

if let == "~":
    n.append(" ")
else:
    for Ke in key_list:
        if let == Ke:
            a = key_code[Ke]
            n.append(final[a])

Proposal

```
from collections import defaultdict
from random import seed, shuffle

def encode(plaintext, key):
"""
Performs a toy-grade substitution cipher on plaintext using the key.
Any characters other than letters and spaces in plaintext will be discarded.
"""
LETTERS = [chr(ord('a') + i) for i in range(26)]
seed(key) # influences shuffle()
code = LETTERS[:] ; shuffle(code)
# letter_code maps LETTERS to a copy of LETTERS that is scrambled using the key;
# unrecognized characters map to the empty string.
letter_code = defaultdict(str, zip(LETTERS, code))
letter_code[' '] = '~'
return ''.join([letter_code[c] for c in plaintext.lower()])

def decode(ciphertext, key):
"""
Performs the inverse of encode().
Any characters other than letters and spaces in ciphertext will be discarded.
"""
LETTERS = [chr(ord('a') + i) for i in range(26)]
seed(key) # influences shu

StackExchange Code Review Q#35259, answer score: 2