patternpythonMinor
Caesar Encryption-Decryption Tool
Viewed 0 times
encryptioncaesartooldecryption
Problem
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 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 decr
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 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 decr
Solution
First off you if we look at the main of your code, it's mostly all the same pattern.
You should make that a function. And then call it.
This severely reduces the amount of code needed.
The way you use this new function is:
If you have not come across
For example, the following are the same:
You should also note that I replaced
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
Also in Python you should use
This is as empty objects are False, and others True. E.g.
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
to do this you need to make a 'proper' function.
Alternately you can use
When calling a function you should always pass it arguments.
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
I would re-write your modes to:
As I recommended you change caesar you will need to change the arguments you pass to it.
You shouldn't add to the
If you need to add more letters then you should add them in the definiton.
If you make
You should change your loop from
Your algorithm can be changed to one line, and
just add letters to
I would discourage
And so I would re-write your code to:
Note: I don't add to
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.
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.
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) - 1You 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'])
TrueNow 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 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 += 26I 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.
Code Snippets
while True:
text = input().upper()
if ...:
break
else:
print()def get_input(message, error_message, fn):
while True:
text = input(message).upper()
if fn(text):
return text
else:
print(error_message)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
)my_fn = lambda a: len(a) - 1
def my_fn(a):
return len(a) - 1>>> bool('')
False
>>> bool('a')
True
>>> bool([])
False
>>> bool(['a'])
TrueContext
StackExchange Code Review Q#110138, answer score: 6
Revisions (0)
No revisions yet.