You should get out of the habit of print
ing results instead of return
ing 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.