Caesar Encryption-Decryption Tool

Question

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()

Answer 1

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(
 'Enter your text : ',
 '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.

• 301_Moved_Permanently: LETTERS is already string.ascii_uppercase no need to redefine it.
• Peilonrayz: @MathiasEttinger I didn't include that as OP didn't use import and has beginner.
• Mahmood Muhammad Nageeb: 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.
• SuperBiasedMan: You could make a lambda to test for integers like this: lambda text: text.strip().isdigit()
• Peilonrayz: @SuperBiasedMan Ah yes, I forgot the function name for isdigit, and chose to be lazy.