Caesar cipher using ASCII character codes

Question

This function should take in a string and a number as parameters and 'rotate' the string to the equivalent alphabet letter that is located at the letter position + 'rotation' parameter.

E.g.: "stackoverflow" --- "fgnpxbiresybj"

def caesar(inputString, rotation = 1):
 zNum = ord('z')
 alphaLen = 26
 result = []
 for c in inputString:
 charNum = ord(c) + rotation
 if charNum > zNum:
 result.append(chr(charNum - alphaLen))
 else:
 result.append(chr(charNum))
 return ''.join(result)

• uli: Not really a programming issue, but Ceasar's Cipher has the property that when it is applied twice, you get the original results back. In other words, the rotation is always 13 and it must wrap around (modulo operation).

Answer 1

PEP 8

The Style Guide for Python Code has many guidelines all Python programs should follow. Marc mentioned one (parameters and variables should be snake_case not bumpyWords.

Another is with regard to keyword parameters: the = should not be surrounded with spaces when used for keywords. Instead of declaring the function like:

def caesar(inputString, rotation = 1):
 ...

it should be:

def caesar(input_string, rotation=1):
 ...

Type hints

Although Python is a not a strongly typed language, it is useful to pretend it is, and provide the expected types for the function parameters and return values. I’ll repeat that: expected types. The declarations actually don’t enforce anything, and are mostly ignored by the Python interpreter, but other tools (pylint, pyflakes, pychecker, mypy, ...) can read in the code and look for inconsistencies based on the declared types.

When using type hints, you do end up putting a space around the = sign for the function’s keyword parameters:

def caesar(input_string: str, rotation: int = 1) -> str:
 ...

Docstrings

Python allows the author to include built-in documentation for functions using a """docstrings""" immediately after the function declaration.

def caesar(input_string: str, rotation: int = 1) -> str:
 """
 Encrypt an input string using a "Caesar Cipher", where each
 alphabetic character is replaced by the alphabetic character
 `rotation` characters after it, wrapping around from `z` back
 to `a`.
 """
 ...

Bugs

As others have noted, the program has bugs which happen when a rotation larger than 25 is used, or non-lowercase letters is given as input.

Efficiency

When translating longer texts, the method used is inefficient. For instance, when translating "stackoverflow", the translation for the letter "o" is computed more than once.

For translation of longer blocks of text, it would be more efficient to determine the translation key once, ahead of time, and then simply looking up the correct translation for each character.

def caesar(input_string: str, rotation: int = 1) -> str:
 """
 Encrypt an input string using a "Caesar Cipher", where each
 alphabetic character is replaced by the alphabetic character
 `rotation` characters after it, wrapping around from `z` back
 to `a`.
 """
 key = {}
 for ch in string.ascii_lowercase:
 key[ch] = chr((ord(ch) - ord('a') + rotation) % 26 + ord('a')
 return ''.join(key.get(ch, ch) for ch in input_string)

Python actually has a built-in translation function for this purpose. Once the key has been created, a translation table may be generated, and used.

def caesar(input_string: str, rotation: int = 1) -> str:
 """
 Encrypt an input string using a "Caesar Cipher", where each
 alphabetic character is replaced by the alphabetic character
 `rotation` characters after it, wrapping around from `z` back
 to `a`.
 """
 key = {}
 for ch in string.ascii_lowercase:
 key[ch] = chr((ord(ch) - ord('a') + rotation) % 26 + ord('a')
 translation_table = str.maketrans(key)
 return input_string.translate(translation_table)

Ideally, this translation_table would only be created once, and could be reused for other translations.

• Peter Mortensen: Pylint can check/enforce some of these.
• Peter Cordes: it would be more efficient to determine the translation key once, ahead of time, and then simply looking up the correct translation for each character. - That's mostly because the Python interpreter is so slow, and .translate could be a compiled C function. If you were writing this in C, it would probably be faster to just use arithmetic, after turning the rotation into a number from 0..25 that allows you to do c += c<thresh ? rotation : rotation - 26;. A good C compiler could even vectorize that to do 16 ASCII bytes at once. (Handle upper/lower with c+= (c|0x20)<thresh ? ...)

Answer 2

Good things

• String building: the new string is built step by step using a list, this is more efficient than concatenating char by char.

To improve

• Bug: caesar('a',500) returns Ȼ. You likely want to use the operator % to stay in the range a-z.
• PEP 8: variable names should be lowercase, with words separated by underscores. More info.

• uber: could you explain how one could use the % in this situation, please?
• Have a nice day: @uber lets say you did caesar('a', 28). You would then do 28%26, which is two. Then you do the shift. Or if you did 256 then you would shift 22 times. Got the idea?
• Roddy of the Frozen Peas: you only want to rotate at most 26 positions, so you use modulo to figure out how much to rotate (0-26) for any given provided positive number. actual_rotation = rotation % 26. for the input of 500, that would result in 6 (500 mod 26 = 6)

Answer 3

I don't think the code is right to transform all characters this way. I think the transform we have should be restricted to lower-case letters, and we should have a similar one (wrapping at 'Z') for upper-case letters.

I guess this shows a shortage of test cases containing upper-case or non-letter characters.

Also, have you tested that this works for rotation values that are negative, or greater than 26? When these tests are added, some work will be required to ensure that they pass, too.