Simple self-made encrypter

The following code:

import random

def keygen():
    l = list('abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" ')
    random.shuffle(l)
    return l

def encrypt(x):
    key = keygen() #or a static key for consistency
    y = list('abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" ')
    z = dict(zip(y, key))
    l = []
    for match in list(x):
        l.append(match.replace(match, z[match]))
    print('key: '+ ''.join(key))
    print('content: '+ ''.join(l))

def decrypt(key, x):
    key = list(key)
    y = list('abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" ')
    z = dict(zip(key, y))
    l = []
    for match in list(x):
        l.append(match.replace(match, z[match]))
    print(''.join(l))

is just a simple self made encrypter that I made for fun, and works like so:

>>> encrypt('hello world how are you')
key: ^lps1|dv)({t&hk#w;=@!6bjrx$4 \[m~"q*i+:-a73?u/5zf%gc9}yn"8_oe]`2
content: v1ttk2bk;ts2vkb2^;12rk!
>>> decrypt('^lps1|dv)({t&hk#w;=@!6bjrx$4 \[m~"q*i+:-a73?u/5zf%gc9}yn"8_oe]`2', 'v1ttk2bk;ts2vkb2^;12rk!')
hello world how are you
>>>

This works pretty well, but I did see one time where there was a single letter that was misplaced. This could have just been due to me not copy/pasting the key in right for decoding, but I am not so sure. Basically does this code look sound? And are there any other methods of doing something like this in a more elegant way?

You should get out of the habit of printing results instead of returning them; if you return the function is reusable. Remember that you can return multiple values with:

return ''.join(key), ''.join(l)

I would personally suggest returning a string directly from keygen.

You don't need to convert y to a list, and probably should name it better (eg. ascii). It would be better as a global constant.

encrypt and decrypt can be unified by making key optional and reversing the arguments when decrypting relative to encrypting; it's symmetric after all.

import random

ASCII = 'abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" '

def keygen():
    l = list(ASCII)
    random.shuffle(l)
    return ''.join(l)

def cipher(x, key=None, backwards=False):
    if key is None:
        key = keygen()

    z = dict(zip(key, ASCII) if backwards else zip(ASCII, key))

    l = []
    for match in list(x):
        l.append(match.replace(match, z[match]))

    return key, ''.join(l)

Since keygen is so easy to call as

encrypt(x, keygen())

I wouldn't make key have a default. This gives the simpler:

import random

ASCII = 'abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" '

def keygen():
    l = list(ASCII)
    random.shuffle(l)
    return ''.join(l)

def cipher(x, key, backwards=False):
    z = dict(zip(key, ASCII) if backwards else zip(ASCII, key))

    l = []
    for match in list(x):
        l.append(match.replace(match, z[match]))

    return key, ''.join(l)

Looking at

l = []
    for match in list(x):
        l.append(match.replace(match, z[match]))

we have a lot of inefficiencies. You should just do

for match in x:

and then

l.append(z[match])

since the replace is a no-op.

This can be a list comprehension:

l = [z[match] for match in x]

This is now just

import random

ASCII = 'abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" '

def keygen():
    l = list(ASCII)
    random.shuffle(l)
    return ''.join(l)

def cipher(x, key, backwards=False):
    z = dict(zip(key, ASCII) if backwards else zip(ASCII, key))

    l = [z[match] for match in x]
    return ''.join(l)

You can then replace some of the challenge with str.translate and str.maketrans:

import random

ASCII = 'abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" '

def keygen():
    l = list(ASCII)
    random.shuffle(l)
    return ''.join(l)

def cipher(x, key, backwards=False, base_chars=ASCII):
    if backwards:
        base_chars, key = key, base_chars

    return x.translate(str.maketrans(base_chars, key))

The bug

There is a duplicate character in ASCII. This can, and does, break things

↓↓
ASCII = 'abcdefghijklmnopqrstuvwxyz123456789!@#$%^&*()_-+={}[]|\:;?/~`"" '
                                                                      ↑↑

Personally, your best best is just to use string.ascii_lowercase + string.digits + string.punctuation, despite it being slightly different to the above. This gives

import random
import string

ASCII = string.ascii_lowercase + string.digits + string.punctuation

def keygen():
    l = list(ASCII)
    random.shuffle(l)
    return ''.join(l)

def cipher(x, key, backwards=False, base_chars=ASCII):
    if backwards:
        base_chars, key = key, base_chars

    return x.translate(str.maketrans(base_chars, key))

Of course, don't roll your own crypto.

StackExchange Code Review Q#77856, answer score: 3