def encrypt(text, key, direction):
if direction == 1: #The direction is either -1, or 1. If it is 1, it goes right. Otherwise, it will go left.
emptys=''
for x in text:
b = ord(x) #b is the individual characters after the ord() function
b+=key
if b<=122:
n = chr(b) #n is the converted letter of the encrypted ASCII number
emptys+=n
else:
o=b-90
q=chr(o)
emptys+=q
return emptys
else:
emptys=''
for x in text:
b = ord(x) #b is the individual characters after the ord() function
b=b-key
if b>=32:
n = chr(b) #n is the converted letter of the encrypted ASCII number
emptys+=n
else:
o=b+90
q=chr(o)
emptys+=q
return emptys
Asked
Active
Viewed 125 times
-2

pvg
- 2,673
- 4
- 17
- 31

Chevy Chen
- 1
- 1
-
5You should format your code, but also: if there is no problem with this code, it does not belong on Stack Overflow. For improvements **and assuming this code works**, you _may_ want to try your luck on [Code Review](http://codereview.stackexchange.com/). – Nelewout Jan 12 '16 at 01:12
-
Your code is too long, the variable names are too short and not enough whitespace between symbols. You should be able to get it down to about half the lines by refactoring the variables that you only use once – John La Rooy Jan 12 '16 at 01:19
1 Answers
0
Your code as written blithely translates alphabetic characters to non-alphabetic and vice versa (e.g. encrypt('abc', 25, 1)
gets 'z!"'
). So it's wrong by most definitions of the traditional Caesar cipher, which should only modify alphabetic characters, and should rotate them within the alphabet.
That said, getting it right is easier than you're making it. The best approach is to avoiding rolling your own rotation code. Python already provides a really nice way to do one-to-one character mappings, str.translate
. For example:
from string import ascii_letters, ascii_uppercase, ascii_lowercase
def encrypt(text, key, direction):
# direction == -1 is trivially converted to direction == 1 case
if direction == -1:
key = 26 - key
# On Py2, you'd use the string module's string.maketrans instead of
# str.maketrans
trans = str.maketrans(ascii_letters, ascii_lowercase[key:] + ascii_lowercase[:key] + ascii_uppercase[key:] + ascii_uppercase[:key])
return text.translate(trans)

ShadowRanger
- 143,180
- 12
- 188
- 271