I've implemented Caesar Cipher with two sample functions, Is my code readable I tried to make it understandable as I can , any comment is appreciated. I am planning to do a Vigenère Cipher soon.
import string
def cipherencryprt (word , key ):
word.lower()
encryptedstring = ""
for i in word :
charvalue = ord(i) +key
if ( charvalue > 122):
charvalue = ord('a') + (charvalue -123)
encryptedstring += chr(charvalue)
else:
encryptedstring += chr(charvalue)
return encryptedstring
def cipherdecrypt(word , key ) :
word.lower()
decryptstring = ""
for i in word :
charvalue = ord(i ) - key
if ( charvalue < 97 ):
charvalue = ord('z') - (96-charvalue)
decryptstring += chr(charvalue)
else: decryptstring+= chr(charvalue)
return decryptstring
if __name__ == "__main__" :
print(cipherencryprt('omar', 4))
print(decrypt(cipherencryprt('omar', 4),4))
2 Answers 2
Line spacing is important for readability. leave two spaces before your functions and don't have so many after. be sure to also be consistent about whitespace around operators.
word.lower()
does not change the case of word but rather returns a string that is the lowercase equivalent of word.The following can be simplified using the modulo operator
charvalue = ord(i) +key if ( charvalue > 122): charvalue = ord('a') + (charvalue -123) encryptedstring += chr(charvalue) else: encryptedstring += chr(charvalue)
becomes
charvalue = ((ord(i) - ord('a')) + key) % 26 + ord('a')
Your
cipherencrypt()
function does not currently support negative keys (see #2), but if it did,cipherdecrypt()
could be written as:def cipherdecrypt(word, key): return cipherencrypt(word, -key)
In python it is recomendend to do this kind of replacement with a comprehesnion:
def cipherencrypt(word, key): return "".join( chr(((ord(i) - ord('a')) + key) % 26 + ord('a')) for i in word.lower() )
Your cipher does not currently support space or any non latin character. If passed a non latin character, like space, your program will not be able to decrypt it. consider expanding your range to all printable characters or adding a special case for space.
Just to add a bit more to what the existing reviews already mentioned.
Magic numbers
if ( charvalue < 97 ):
When i read this i'm left wondering what char is 97
? So i either trust my gut (and possibly interpret it incorrectly) or i have to go check the ascii table (which i did).
Just use the char value directly, which is a lot more readable:
if ( charvalue < 'a' ):
Unnecessary parenthesis
Following up on the previous example:
if ( charvalue < 'a' ):
These extra parenthesis add nothing to the code. Most often, these are left overs of other languages that force them. In python you don't need them.
So you can instead write:
if charvalue < 'a':
Unused imports
Note that you are importing string
up top:
import string
Yet, there is nothing you are using from this module. Remember that ord
and chr
are built in functions and don't come from the string
module.
Always pay close attention to the unused modules and remove them.
Naming convention
For function and variable names the convention is as follows:
Function names should be lowercase, with words separated by underscores as necessary to improve readability.
To enumerate a couple of examples in the code:
cipherdecrypt
should becipher_decrypt
cipherencryprt
should becipher_encrypt
. Also interesting to note that there was a typo in this one.
As a side note, the typos are actually more important than they seem, because they considerably decrease readability. Depending on the type of the typo, someone else can have a hard time figuring out what it means, in he/she is not in the context.
Consistent Indentation
Take a look at these two else
clauses:
else:
encryptedstring += chr(charvalue)
Versus this one
else: decryptstring+= chr(charvalue)
They are indented differently. Being consistent brings harmony to the code style you are following, and makes it easier for anyone to follow along. The preferred indent style is the first example where the statement comes in the line below and indented properly.
Speaking of consistency, the names variable names are also not consistent in the wording. You have encrypted
vs decrypt
. This difference would have been more noticeable if they had been named according to the naming scheme mentioned earlier.
Explore related questions
See similar questions with these tags.