Search for a Pythonic Caesar

Update

I've posted a follow up question to this code. Find it here:

Not a very interesting implementation of a Simple Substitution Cipher in Python.

I implemented a second encryption algorithm: the well known caesar-cipher. This turned out to be longer than I expected. The "main" code is very concise, but the interface that followed seems a bit un-pythonic. Here it is:

caesar.py

```
"""
This python file contains the "shift()" method to encrypt as well as decrypt
strings.

Help on command line arguments:

1. Use without any of those to enter an interactive mode.
2. Pass only the plaintext to encrypt using default settings (shift of 97

with alphabet of all printable ascii characters).

1. Pass the shift amount (positive to encrypt, negative to decrypt) and one

of "lower", "upper" or "all" to choose the alphabet.
"""
import string
import sys

def shift(message, amount=97, alphabet=string.printable):
"""
This method takes a string and shifts it by the specified amount. A
positive amount shifts the characters up, while a negative amount shifts
the characters down.

:param message: The message to be encrypted.
:param amount: The amount to shift the message by. (Default is 97)
:param alphabet: The alphabet for determining the result of a shift.
(Default is all printable characters)
:return:
"""
# This handles all the "case issues"
if alphabet == string.ascii_lowercase:
message = str.lower(message)
elif alphabet == string.ascii_uppercase:
message = str.upper(message)
shifted_alphabet = alphabet[amount:] + alphabet[:amount]
table = str.maketrans(alphabet, shifted_alphabet)
return message.translate(table)

if __name__ == '__main__':
argument_count = len(sys.argv)
text = ''
offset = 97
if argument_count == 1:
# The script is being used independently, so display menus and stuff
print('\n\n'
'+---------------------------

I started by running your program, and poking it a bit to see what happens if I misbehave. This is a good way to catch errors and edge cases. Here’s what I found:

-
ValueError if I give bad data for “Please enter the shift amount”.

Examples:

Please enter the shift amount (negative to decrypt) :
Traceback (most recent call last):
File "caesarcr.py", line 49, in
offset = int(input('Please enter the shift amount (negative to '
ValueError: invalid literal for int() with base 10: ''


Please enter the shift amount (negative to decrypt) : notanumber
Traceback (most recent call last):
File "caesarcr.py", line 49, in
offset = int(input('Please enter the shift amount (negative to '
ValueError: invalid literal for int() with base 10: 'notanumber'


You should catch the ValueError, and either:

• Default the shift if I enter nonsense, or

• Keep prompting me until I give valid input, or

• End the program with an error message

But a traceback is rarely a good form of error for an end-user.

-
No output if I enter a number that isn’t 1 or 2 at “choice number”.

I can repeat this with any choice of number; I just don’t get any output:

Please input the text to be encrypted : hello world
Please enter the shift amount (negative to decrypt) : 3
The alphabet sets :

1. Lowercase ascii
2. Uppercase ascii

Please enter the choice number, or anything else to choose default alphabet
(all printable ascii characters): 3
The encrypted text is :


The problem is these branches here (comments mine):

if str.isnumeric(choice):
    number = int(choice)
    if number == 1:
        # do stuff with lowercase ASCII
    elif number == 2:
        # do stuff with uppercase ASCII
  # else:
  #     What about input that falls down here?
else:
    # do stuff with mixed case

You need to add that missing else: branch.

-
I get unchanged output if I use offset ≥ 100.

Some examples:

$ python3 caesarcr.py "hello world" 99 all
gdkkn~vnqkc

$ python3 caesarcr.py "hello world" 100 all
hello world

$ python3 caesarcr.py "hello world" 101 all
hello world


I suspect this is related to the fact that the length of string.printable is 100, and you’re failing to wrap around correctly. Perhaps it’s okay that shifting by 100 gives the same output, but after that it should reset – so offset by 101 is the same as offset by 1, etc.

Playtime over, time to dive into the code. A few items of note:

-
Inside shift(), your calls to str.upper() and str.lower() can be rewritten as:

message = message.lower()
message = message.upper()

That’s the more approach I see more often.

-
I think this line is the source of your wrap-around woes:

shifted_alphabet = alphabet[amount:] + alphabet[:amount]

You should probably set amount to be amount % len(alphabet) before this line, then I think you’d be okay.

-
I’m not sure why 97 is the default shift length, or if it’s even appropriate to have a default shift. At the very least, there should be a comment to explain this choice.

-
It’s not clear why I can’t supply two arguments, say:

$ python3 caesarcr.py 'hello world' 3

and have the script automatically use the full set of ASCII characters for me.

-
To break up the main() block, I’d wrap the interactive and command-line help in two different functions. Then you’d have:

if __name__ == '__main__':
    if len(sys.argv) == 1:
        interactive_encrypt()
    elif len(sys.argv) <= 4:
        command_line_encrypt()
    else:
        print_usage_message()

Within the command-line parser, you can tidy it up slightly by combining cases. For example:

text = sys.argv[1]

if len(sys.argv) >= 2:
    offset = int(sys.argv[2])
else:
    offset = 97

if len(sys.argv) >= 3:
    mode = sys.argv[3]
else:
    mode = 'universe'

rather than, for example, getting text = sys.argv[1] for each length individually.

StackExchange Code Review Q#112121, answer score: 8