# Caesar Encryption-Decryption Tool

I'm a beginner in Python. I wrote this code which encrypts and decrypts text using Caesar chipher. I wrote it in many days and optimized it many times. But how it can be improved more?

I've used lists because they are flexible and they serve my algorithm.

print('''# This is version 1.0 Caesar Encryption-Decryption Tool.
# This tool is written by Mahmoud Naguib and it will be improved.
# You can contact me at : https://www.facebook.com/naguibarea
-----------------------------------------------------------------''')
letters_table = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q',
'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z']

def caesar():
# This line iterates through every letter in the string.
for letter in range(len(text)):
# If there is a letter which isn't an English alphabet.
if not text[letter].isalpha():
letters_table.append(text[letter])
# This line locates the letter's numeric value in other words its position.
position = letters_table.index(text[letter])
# This line adds the key entered by the user to the letter's
# numeric value to reach the new letter.
step = position + key
# if the encrypted letter's numeric value is greater than 25
# then its new numeric value is the old numeric valuu %
# the number of English alphabet letters.
if step > 25:
step %= 26
# If step is lower than 0 then we need to add 26 to it to complete the decryption process.
if step < 0:
step += 26
# If position is greater than 25 that means it's not an alphabet
# value and it will be printed without adding the key(the letter will be printed as it is).
if position > 25:
step = position
# This line locates the letter's numeric value(its position) and prints the new letter.
# end is a keyword argument to combine all letters together.
print(letters_table[step], end='')
print('\n' + '-----------------------------------------------------------------')

while True:
while True:
# This line ask the user if he wants to encrypt or decrypt a text.
mode = input('Type E for enryption mode or D for decryption mode : ').upper()
if mode == 'D' or mode == 'E':
break
else:
print('You must enter E or D ! ')
# This line takes the user's text and converts it to uppercase to search for it in the letter's table.
while True:
text = input('Enter your text : ').upper()
if len(text) != 0:
break
else:
print('You can not leave it blank ! ')
while True:
try:
# This line takes the user's key.
key = int(input('Enter a key : '))
if type(key) == int:
break
except ValueError:
print('You must enter an integer number ! ')
# If the user wants to encrypt a text it will call the caesar() function.
if mode == 'E':
caesar()
# If the user wants to decrypt a text it will make the key value negative so that there's a reverse process
# and it will call the caesar() function.
elif mode == 'D':
key = 0 - key
caesar()


First off you if we look at the main of your code, it's mostly all the same pattern.

while True:
text = input().upper()
if ...:
break
else:
print()


You should make that a function. And then call it.

def get_input(message, error_message, fn):
while True:
text = input(message).upper()
if fn(text):
return text
else:
print(error_message)


This severely reduces the amount of code needed. The way you use this new function is:

mode = get_input(
'Type E for enryption mode or D for decryption mode : ',
'You must enter E or D ! ',
lambda mode: mode in 'DE'
)

text = get_input(
'You can not leave it blank ! ',
lambda text: text
)


If you have not come across lambda before it's a way to make small functions. For example, the following are the same:

my_fn = lambda a: len(a) - 1

def my_fn(a):
return len(a) - 1


You should also note that I replaced mode == 'D' or mode == 'E' to mode in 'DE'. It's much easier to add to this if you need to. So if you add say 'A' as a function, you can change it to mode in 'ADE'.

Also in Python you should use if text rather than if len(text) != 0. This is as empty objects are False, and others True. E.g.

>>> bool('')
False
>>> bool('a')
True
>>> bool([])
False
>>> bool(['a'])
True


Now onto the bit that is a bit more challenging.
To make a function to check if you can change the input into a number, using int, we can't use lambda. to do this you need to make a 'proper' function.
Alternately you can use str.isdigit. (Thanks @SuperBiasedMan)

def int_able(string):
try:
int(string)
return True
except ValueError:
return False

key = int(get_input(
'Enter a key : ',
'You must enter an integer number ! ',
int_able
))

# Or
key = int(get_input(
'Enter a key : ',
'You must enter an integer number ! ',
lambda text: text.strip().isdigit()
))


When calling a function you should always pass it arguments. caesar() is a bad design. For a simple script like this it's alright, however it's not a good habit to get into. Instead, I would recommend that you call it with caesar(text, key).
I would re-write your modes to:

if mode == 'D':
key = -key

caesar(text, key)


As I recommended you change caesar you will need to change the arguments you pass to it.

You shouldn't add to the letters_table, that should stay the same. If you need to add more letters then you should add them in the definiton.

If you make letters_table a constant, then you can change it to a string.

You should change your loop from for letter in range(len(text)) to for letter in text.

Your algorithm can be changed to one line, and if step < 0 should never happen.

And you should probably remove the if position > 25 it's not a good idea, just add letters to letters_table.

I would discourage print(..., end=''), instead make a string.

And so I would re-write your code to:

LETTERS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'

def caesar(message, key):
caesar_text = ''
for letter in message:
caesar_text += LETTERS[(LETTERS.index(letter) + key) % 26]

print(caesar_text)


Note: I don't add to LETTERS if you need to add more then hard-write them in.

To note, I didn't read your comments, there were to many, and not meaning to be rude, but I can read Python, in fact it's more straightforward than English at times.

# If step is lower than 0 then we need to add 26,
# to it to complete the decryption process.
if step < 0:
step += 26


I know which is easier, and faster, to read by far.

But all in all you have nice code, reduce the amount of comments, and pass argument and you'll do great.

• LETTERS is already string.ascii_uppercase no need to redefine it. Nov 8, 2015 at 5:44
• @MathiasEttinger I didn't include that as OP didn't use import and has beginner. Nov 8, 2015 at 11:35
• Thank you! You've written a great answer. I'm working on knowing more about concepts I don't know and developing the code more accroding to your suggestions. Nov 8, 2015 at 14:35
• You could make a lambda to test for integers like this: lambda text: text.strip().isdigit() Nov 9, 2015 at 9:55
• @SuperBiasedMan Ah yes, I forgot the function name for isdigit, and chose to be lazy. Nov 9, 2015 at 9:58