Python Caesar Cipher

Question

I am a student doing their AS Computing course and I have created a piece of code in which a message can be typed and then scrambled by moving letters along the alphabet. I was wondering if anyone knew any way in which this could be improved.

while(True):
 message = input("What would you like the message to say? ").lower()
 newmessage = ""
 for i in range(0,26):
 for letter in message:
 decimal = (ord(letter))
 decimal = decimal + i
 if decimal > 122:
 decimal = decimal -26
 elif decimal < 97:
 decimal = decimal + 26
 newmessage += (chr(decimal))
 print (newmessage)
 newmessage = ""

Answer 1

Here are some comments to your code, which will be followed by an alternate implementation:

• Use functions to do the rotation – If you have learned about functions I would strongly suggest to use functions to do the actual rotation. You call it with the string to rotate and rotation offset, and you'll get the rotated string in return.
• Avoid magic numbers – In your code you use magic numbers like 122 and 97, your code looks cleaner if you replace these by what they actually are: ord('z') and ord('a'). Alternatively use constants like ALPHABET_LENGTH = 26, but I'll come back to this one
• Don't hide reserved words or modules – decimal is a module in Python, and to use that as a variable name is considered bad practice.
• Naming scheme dictates to use snake_case – Instead of newmessage it is recommended to use new_message and similar. This reads more easy.
• Be consistent on indentation – You use a varying amount of spaces to indent your code, please be consistent and use 4 spaces. The previous and this item is related to conforming to the PEP8 guidelines. Please read and follow these guidelines, as that is quite common when using Python.

Algorithm considerations

If keeping to your format of not using functions, there are still some issues related to your algorithmic choices:

• What about letters not in the lowercase alphabet? – You don't handle punctuation chracters and non-alpha characters so these would leave somewhat strange behaviour
• You don't have a means to exit your code – You are essential having an eternal loop, leaving you to kill the program execution as only means of leaving it
• Range checks can be joined in Python – Instead of doing one lower than and one larger than you could do 97 < var < 122 to verify a complete range (This will make better sense in code example below)
• range() default to start at 0 – That is range(0, 26) and range(26) is the same, and the latter is preferred for clarity
• You don't need to do (expr), unless you want line continuation – It is better to write new_message += chr(decimal) without the extra parentheses. You could use them if you want to spread the expression within over multiple lines, but normally keep the expression without.

Adhering to my style and algorithmic considerations we arrive at this code:

ALPHABET_LENGTH = 26
while(True):
 message = input("What would you like the message to say? ").lower()
 if message == "quit":
 break
 for i in range(ALPHABET_LENGTH):
 new_message = ""
 for letter in message:
 if 'a' <= letter <= 'z':
 char_pos = ord(letter) - ord('a')
 # Rotate by i, and make sure we're within alphabet
 char_pos = (char_pos + i) % ALPHABET_LENGTH
 new_message += chr(char_pos + ord('a'))
 # new_message += chr((ord(letter) - ord('a') + i) % ALPHABET_LENGTH + ord('a'))
 else:
 new_message += letter
 print (new_message)

Within the rotation block there is a commented line which could replace all of the other code, but it is not so easy to read and understand as the uncommented code.

Alternate implementation with functions

In general I would rather use functions to do stuff like this, and I would encapsulate my entire program within a if __name__ == '__main__': block to allow for module execution. That is that the script could be imported as a module, and you could call the different functions to add caesar-cipher rotation to other programs of yours.

One version of this could be like this:

from string import ascii_lowercase
def rotate_lowercase_char(char, offset):
 """Return lowercase <char> rotated by <offset> characters.
 If <char> is a lowercase character rotate it, or else return
 it unchanged allowing for non-alpha characters to be present."""
 if not char in ascii_lowercase:
 return char
 else:
 position = ascii_lowercase.index(char)
 position = (position + offset) % len(ascii_lowercase)
 return ascii_lowercase[position]
def rotate_lowercase_text(text, offset):
 """Return <text> rotated by <offset> characters."""
 return ''.join(rotate_lowercase_char(letter, offset) for letter in text)
def main():
 while(True):
 message = input("What would you like the message to say? ").lower()
 if message == "quit":
 break
 for i in range(len(ascii_lowercase)):
 print("Offset: {}, Rotated: {}".format(i, rotate_lowercase_text(message, i)))
if __name__ == '__main__':
 main()

This uses string.ascii_lowercase for the set of lowercase letters and a positional approach for rotating a letter a given approach. The methods could be extended using default parameters to allow for other characters sets, but that is left as an exercise to the reader.

Furthermore I've presented two functions here one for rotating the entire string, and one for rotating a single character. These could have been combined, but I like the single responsibility principle of letting one function doing exactly one thing.

Lastly I've kept the main logic from the original post, only embedded it into a main(), and added some extra information related to the output to display the various rotation offsets used for presenting the text.

For even more various alternative implementation look at other posts here at Code Review tagged caesar-cipher (in a variety of languages). Both shorter and longer and simpler and more complex variation exists.

Answer 2

You can make it two lines, just by doing the rollover as a modulo:

while(True):
 print ''.join( [ chr(((ord(letter) - 96) % 26)+97) for letter in input("What would you like the message to say? ").lower() ] )