-2
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
pvg
  • 2,673
  • 4
  • 17
  • 31
  • 5
    You 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 Answers1

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