HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple Caesar shift and deshift

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
shiftsimplecaesardeshiftand

Problem

I wrote this program to do a simple Caesar shift by a user inputted key, and then deshift.
I'm really enjoying it, but I've run out of ideas on improvements! Can you think of anything?

def decrypt():
    a=raw_input("Give me the word to decrypt:")
    number=input("What was it shifted by?")
    b=list(a)
    str(b)
    c=[ord(x)for x in(b)]
    d=[]
    for i in c:
        d.append(i-number)
    e=[chr(i) for i in (d)]
    e="".join(e)
    print "Decryption Successful, your word is",e,"!"

def encrypt():
    a=raw_input("Give me a word:")
    number=input("Give me a number:")
    b=list(a)
    str(b)
    c=[ord(x)for x in(b)]
    d=[]
    for i in c:
        d.append(i+number)
    e=[chr(i) for i in (d)]
    e="".join(e)
    print "Your Caesar shifted result (ascii code) is:",e,"!"
    print "Your key is", number, ",remember that!"

def menu():    
    print "\n\n\nWelcome to the Caesar Shifter."
    print "What would you like to do?"
    print "Option 1:Encrypt Word"
    print "Option 2:Decrypt Word"
    print "If you would like to quit, press 0."
    choice=input("Pick your selection:")
    if choice==1:
        run=encrypt()
        run
        menu()
    elif choice==2:
        derun=decrypt()
        derun
        menu()
    elif choice==0:
        quit
    else:
        print"That is not a correct selection, please pick either 1, or 2."

menu()

Solution

This might not be quite what you've got in mind, but one big improvement you could make would be to use meaningful variable names and insert whitespace.

def decrypt():
    cyphertext = raw_input('Give me the word to decrypt:')
    shift = input('What was it shifted by?')

    cypher_chars = list(cyphertext)
    str(b) # this does nothing; you should remove it
    cypher_ords = [ord(x) for x in cypher_list]
    plaintext_ords = []
    for i in cypher_ords:
        plaintext_ords.append(i - shift)

    plaintext_chars = [chr(i) for i in plaintext_ords]
    plaintext = ''.join(plaintext_chars)
    print 'Decryption Successful, your word is', plaintext, '!'


Of course you could actually compress much of this into a one-liner. But for a new programmer, I'd suggest sticking with readable variable names that make it clear what's going on.

Still, you could do that while compressing the code a bit:

def decrypt():
    cyphertext = raw_input('Give me the word to decrypt:')
    shift = input('What was it shifted by?')

    cypher_ords = [ord(x) for x in cyphertext]
    plaintext_ords = [o - shift for o in cypher_ords]
    plaintext_chars = [chr(i) for i in plaintext_ords]
    plaintext = ''.join(plaintext_chars)
    print 'Decryption Successful, your word is', plaintext, '!'

Code Snippets

def decrypt():
    cyphertext = raw_input('Give me the word to decrypt:')
    shift = input('What was it shifted by?')

    cypher_chars = list(cyphertext)
    str(b) # this does nothing; you should remove it
    cypher_ords = [ord(x) for x in cypher_list]
    plaintext_ords = []
    for i in cypher_ords:
        plaintext_ords.append(i - shift)

    plaintext_chars = [chr(i) for i in plaintext_ords]
    plaintext = ''.join(plaintext_chars)
    print 'Decryption Successful, your word is', plaintext, '!'
def decrypt():
    cyphertext = raw_input('Give me the word to decrypt:')
    shift = input('What was it shifted by?')

    cypher_ords = [ord(x) for x in cyphertext]
    plaintext_ords = [o - shift for o in cypher_ords]
    plaintext_chars = [chr(i) for i in plaintext_ords]
    plaintext = ''.join(plaintext_chars)
    print 'Decryption Successful, your word is', plaintext, '!'

Context

StackExchange Code Review Q#3201, answer score: 8

Revisions (0)

No revisions yet.