2
\$\begingroup\$

I have just written a simple program to implement the Caesar cipher using lists. The program assumes the input will be all capital letters. I'm quite new to Python and am just curious of some more efficient ways to write this code. Honestly I just want to work on defining functions and all that jazz.

print("This encryption program uses the Caesar cipher to encrypt your message.")
mode = input("E to Encrypt or D to Decrypt")
if mode == "E":
    string = input("Input your message to encrypt in all capital letters: ")
    key = int(input("Create your encryption key (remember this)"))
    length = len(string)

    old_values = [0]*length   
    new_values = [0]*length

    olds_position = 0         
    news_position = 0

    for char in string:                        
        old_values[olds_position] = ord(char)  
        olds_position += 1                     

    for value in old_values:
        if value != 32:        
            new_values[news_position] = (value + key)
        else:
            new_values[news_position] = value
        news_position += 1

    for newvalue in new_values:
        if newvalue != 32:  
            if newvalue > ord('Z'):
                newvalue -= 26
            elif newvalue < ord('A'):
                newvalue += 26
            print(chr(newvalue)+"", end="")
        else:
            print(" ", end="")

if mode == "D":
    key = int(input("Enter your encryption key: "))
    string = input("Input your encrypted message to descrypt it: ")
    length = len(string)

    old_values = [0]*length   
    new_values = [0]*length

    olds_position = 0         
    news_position = 0

    for char in string:                        
        old_values[olds_position] = ord(char)  
        olds_position += 1                     

    for value in old_values:
        if value != 32:        
            new_values[news_position] = (value - key)
        else:
            new_values[news_position] = value
        news_position += 1

    for newvalue in new_values:
        if newvalue != 32:  
            if newvalue > ord('Z'):
                newvalue -= 26
            elif newvalue < ord('A'):
                newvalue += 26
            print(chr(newvalue)+"", end="")
        else:
            print(" ", end="")
\$\endgroup\$
3
\$\begingroup\$

Here are some changes that you could make to your code. I'll explain various parts of the script below.

import sys

mode = input("E to encrypt, D to decrypt: ").upper()

if mode == 'E':
    string = input("Input your message to encrypt : ").upper()
    def op(x, y): return x + y 
elif mode == 'D':
    string = input("Input your message to decrypt : ").upper()
    def op(x, y): return x - y
else:
    print "Please enter either D or E for mode option. Terminating script!"
    sys.exit()

key = int(input("Create your encryption key (remember this): "))

ord_space = ord(' ')
ord_A = ord('A')
ord_Z = ord('Z')
alpha_num = 26 

def inner_func(value):
    newvalue = op(value, key) if value != ord_space else value
    return newvalue

def outer_func(value):
    newvalue = (value - alpha_num) if value > ord_Z else ( value + alpha_num if value < ord_A else value)
    return newvalue

text = [chr(outer_func(inner_func(ord(char)))) for char in string]

Checking User Input

  • The first thing I do is import the Python module sys. At the beginning of your program you ask the user to enter E or D depending on whether they want to encrypt or decrypt their string. In your programs current form it isn't robust to the case where the user inputs some symbol(s) that isn't equivalent to E or D. So after taking the users mode input I check that it is either E or D and if it isn't I exit the program by calling sys.exit().
  • The next way I help the user (and in turn help our program) is to automatically transform their input to upper case charachters by calling .upper() in every place where you take user input.
  • Question: Are there any other user inputs we should check? (Hint: Key)

Taking Advantage of Object Scope

  • Notice that I'm defining string and the function op within the first if-elif statement. I'm then reusing these objects inside of functions later on without having to pass them in as parameters. Within each function Python will search 1st for a local object (in this case op) and then if there is no object with the given name inside of the function it will search the global namespace of the program. In this case because op is defined outside of a function, it belongs to the global namespace. The above also applies to ord_space, ord_A, etc, which again are also used inside the functions inner_func and outer_func. A more detailed description of Python scoping rules can be found here.

Conditional Expressions

  • Within inner_func and outer_func I'm using what is known in Python as a conditional expression. In other programming languages such as C++ they are also called ternary operators. As explanation consider: x = a if foo else b. The variable x will be assigned the value of a if the statement foo evaluates to true. If foo is false then x will be assigned the value of b. Conditional expressions often lead to more concise code which in the end will lead to less bugs and easier maintenance. Admittedly in outer_func I went a bit overboard and chained two of them together, e.g. x = a if foo else ( b if bar else c ). I wouldn't recommend doing this generally but I thought I'd demonstrate the possibility just for fun.

Tying it all together: List Comprehensions

  • Finally, on the last line of the code I've demonstrated one of the cornerstones of Python, list comprehensions. All a list comprehension does is build a list by looping over the elements of another container and applying some operator to them. For example if we had a list x = [1, 2, 3] and we wanted to add 1 to each element in x while at the same time creating a new list y we could do it like so:

    y = [i + 1 for i in x]
    

This code has the same effect as the following:

    y = [0] * len(x)
    counter = 0
    for i in x:
        y[counter] = i + 1
        counter = counter + 1
\$\endgroup\$
  • \$\begingroup\$ I think those two functions can even be replaced using the modulo function (% operator). Something like: ord('A')+(char-ord('A')%26. Would be nice to define ord('A') and 26 as constants to avoid magic numbers. As a more educational comment for the OP: as an alternative to comprehensions you could also use the map function, although comprehensions are definitely more 'Pythonic'. \$\endgroup\$ – agtoever Oct 19 '14 at 6:38
1
\$\begingroup\$
  • Your code is pretty complicated because you work on ASCII values. In my opinion a good programme is based on real world objects. What you did with ASCII values can be made more simply with the alphabet.

  • What if you find a bug? Fixing a bug in an imperative style can give an headache, refactor your code into functions.

  • What about invalid input? Crashing with a weird error message when the input is bad for sure is not user friendly. Add a myformatting(text) function to delete any digits or punctuation.

  • Your code is so retrò, I know that writing a GUI can take some time and be a little confusing but your programme will than become much nicer.

  • Personally I woud not define functions inside if statements. Just define encrypt(text,key) and decrypt(text,key). You will than set a variable and branch depending on the variable

I am now going to give you some code, but please don't copy this code right away, you can learn much more by trying to rewrite your code by yourself following my suggestions. Also this code has no user interface, you can write one by yourself as you like it most.

import string

ALPHABET = string.ascii_lowercase
PUNCTUATION = string.punctuation + string.whitespace + string.digits

def cycle_get(lst, index):
    return lst[index % len(lst)]

def my_formatting(string):
    for i in string:                                     
        if i in PUNCTUATION: 
            string = string.replace(i,'')               
    string = string.lower()
    return(string)

def encode(string, key):
    new_string = ""
    string = my_formatting(string)
    for i in string:
        if i == " ":
            new_string += " "
        else:
            position = ALPHABET.index(i)
            new_string += cycle_get(ALPHABET, position + key)
    return(new_string)

def decode(string, key,):
    return(encode(string, -key))
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.