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

Caesar Encryption Tool

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

Problem

I'm newbie in Python and I wrote a tool called Caesar Encryption Tool. At the moment it only encrypts. It takes a plain text and a key from the user then encrypts the plain text using the key. How can this code be improved?

```
# This is version 1.0 Caesar Encryption Tool and it will be improved.
# This tool is written by Mahmoud Naguib.
# My facebook account is at : https:\\www.facebook.com\naguibarea
# This is letters table.
lettersTable = ['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']
# This line takes the user's key it's 3 by default.
key = int(input('Enter your key : '))
# This line takes the user's plain text and converts it
# to uppercase to search for it in the English alphabet.
plainText = input('Enter your plain text : ').upper()
for letter in range(len(plainText)):
# If there is a letter which isn't alphabet like numbers,
# special characters and spaces it will be appended to the list.
if plainText[letter] not in lettersTable:
lettersTable.append(plainText[letter])
# This line locates the plain letter's numeric value in other words its position.
position = lettersTable.index(plainText[letter])
# This line adds the key entered by the user to the plain letter's
# numeric value to reach the new encrypted 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 position is greater than 25 that means it's not an alphabet
# value and it will be printed without adding the key.
if position > 25:
step = position
# This line locates the encrypted letter's numeric value in other words
# its position and prints the encrypted letter.
# end is a keyword argument to combine all letters together.
print(letter

Solution

Welcome to Code Review.

I would be going through this code quickly.

  • Read PEP 8. It is de facto standard for coding in python. Generally, we name the variables in lower_case_with_underscore, and any constant used as CONSTANT_UNDERSCORE.



  • Use in-built functions. You are using your letters_table to find out whether the character is alphabet or not, use isalpha() instead. Similarly you can use ord and chr functions which would allow you to preserve case. Using all of these functions would remove the need of letters_table



  • Keep consistent comments. Your comments indicated that key default value is 3. It is not, and if I leave input blank, it would raise error.



  • Make your code modular. You can easily separate your I/O logic with implementation logic. (Use functions)



Since you indicate you want to improve this code yourself, I would refrain from posting refactored version of your code. (If you want me to as an example, I can do that).

Here are some more improvements which you could incorporate:

  • Allow user an option to select between encryption and decryption mode. (with functions, this would be done by having an extra parameter with default value)



  • Other encryption logic while maintaining the same I/O logic. (Would help you understand power of function modules)



Some Python code quality skills which might be handy in future:

  • Write useful docstring (see examples in link). While for this


program, your comments are more than enough, it is useful skill.

  • Learn to write some assertion tests and then unit tests.

Context

StackExchange Code Review Q#107135, answer score: 3

Revisions (0)

No revisions yet.