Caesar Cipher in Python 3

Question

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

Answer 1

1. 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.

2. word.lower() does not change the case of word but rather returns a string that is the lowercase equivalent of word.

3. 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')
   

4. 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)
   

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

6. 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.

Answer 2

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 be cipher_decrypt
• cipherencryprt should be cipher_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.