2
\$\begingroup\$

I've written a Caesar Cipher in python (a more in-depth explanation is provided in the docstring at the top of the program). At first, it was supposed to just be the cipher, but I expanded it to have the user guess the original string, given the encoded string. I would like feedback on the caesar algorithm, and any other nitpicks you can find in my program. Any and all feedback is appreciated and considered!

Link to github prompt

cipher.py

""" Import Statements """
import random

"""
Caesar cipher - Implement a Caesar cipher, both encoding and decoding.

The key is an integer from 1 to 25. This cipher rotates the letters of the alphabet
(A to Z). The encoding replaces each letter with the 1st to 25th next letter in the
alphabet (wrapping Z to A). So key 2 encrypts "HI" to "JK", but key 20 encrypts "HI"
to "BC". This simple "monoalphabetic substitution cipher" provides almost no security,
because an attacker who has the encoded message can either use frequency analysis to
guess the key, or just try all 25 keys.

Input is all lowercase, so using ord() should use 97-122 a-z
"""

WORDS = [
    "thisisarandomstring", "hellomynameisben", "whatisyourname", "thiswaswrittenonamac",
    "macsarethebestcomputer", "thisprogramisinpython", "thisonlyhasletters", "andeveryother",
    "stringinthislist", "isonlyusingletters", "becausethatshowitworks", "andifyoudontlikeit",
    "thenyoucangetyour", "naysayerselfoutofhere", "thatshowiroll"
]

CHART = {
    97: 'a', 98: 'b', 99: 'c', 100: 'd', 101: 'e', 102: 'f', 103: 'g',
    104: 'h', 105: 'i', 106: 'j', 107: 'k', 108: 'l', 109: 'm', 110: 'n',
    111: 'o', 112: 'p', 113: 'q', 114: 'r', 115: 's', 116: 't', 117: 'u',
    118: 'v', 119: 'w', 120: 'x', 121: 'y', 122: 'z'
}

def encode(text, rotations):
    """ Encodes the passed text, rotating `rotations` times """
    encoded_text = ""
    for index in range(len(text)):
        new_position = ord(text[index]) + (rotations % 26)
        if new_position > 122:
            new_position = 97 + (new_position - 122)
        encoded_text += CHART[new_position]
    return encoded_text

def decode(text, rotations):
    """ Decodes the passed text, with the passed rotations """
    decoded_text = ""
    for index in range(len(text)):
        new_position = ord(text[index]) - (rotations % 26)
        if new_position < 97:
            diff = 97 - new_position
            new_position = 122 - diff
        decoded_text += CHART[new_position]
    return decoded_text

def check_guess(guess):
    """ Checks how close the guess is to the original string """
    if len(guess) != len(DECODED):
        return "Guess must be same length to check guess!"
    response = ""
    for i in range(len(guess)):
        if guess[i] == DECODED[i]:
            response += guess[i]
        else:
            response += " "
    return response


if __name__ == '__main__':

    ROTATIONS = random.randint(3, 26)
    ENCODED = encode(random.choice(WORDS), ROTATIONS)
    DECODED = decode(ENCODED, ROTATIONS)
    print(f"""
        Encoded: {ENCODED}
        Length: {len(ENCODED)}
    """)

    print(f"What is the original string?")
    GUESS = input(">>> ")
    GUESSES = 1
    while GUESS != DECODED:
        print(f">>> {check_guess(GUESS)}")
        GUESS = input(">>> ")
        GUESSES += 1

    print("Correct!")

    print(f"""
        Encoded: {ENCODED}
        Original: {DECODED}
        Guesses: {GUESSES}
    """)
\$\endgroup\$
2
\$\begingroup\$

First comment - "Import Statements" - is unnecessary.


I would add few empty lines to make code more readable - ie. before return, for


Document PEP 8 -- Style Guide for Python Code suggest to use UPPER_CASE_NAMES for constant values - WORDS and CHARS you can treat as constant values but ROTATIONS, ENCODED, DECODED, GUESS,GUESSES` are not constant values.


Instead of range(len(..)) in

for index in range(len(text)):
    new_position = ord(text[index]) - (rotations % 26)

you can iterate list

for char in text:
    new_position = ord(char) - (rotations % 26)

There is similar situation in other loop.

for i in range(len(guess)):
    if guess[i] == DECODED[i]:
        response += guess[i]

But here you need i to get DECODED so you can use enumerate()

 for i, char in enumerate(guess):
    if char == DECODED[i]:
        response += char
    else:
        response += " "

You can calculate rotations % 26 only once - before for loop.

\$\endgroup\$
1
\$\begingroup\$

Instead of using a CHART[new_position], I'd recommend using chr(new_position) function instead. Good thing is you're using odr() already.

Your functions now look like [also as the first answer says, made some changes in way of iterating over elements because you don't need the index anyway]:

def encode(text, rotations):
'''
Encodes the passed text, rotating `rotations` times
'''
encoded_text = ""
for character in text:
    new_position = ord(character) + (rotations % 26)
    if new_position > 122:
        new_position = 97 + (new_position - 122)
    encoded_text += chr(new_position)
return encoded_text

def decode(text, rotations):
'''
Decodes the passed text, with the passed rotations 
'''
decoded_text = ""
for character in text:
    new_position = ord(character) - (rotations % 26)
    if new_position < 97:
        diff = 97 - new_position
        new_position = 122 - diff
    decoded_text += chr(new_position)
return decoded_text

Not a big deal but you can also pass (rotations % 26) as a third parameter to encode and decode function and calculate (rotations % 26) in the main function. So that your functions look like:

def encode(text, rotations, rem):
'''
Encodes the passed text, rotating `rotations` times
'''
encoded_text = ""
for character in text:
    new_position = ord(character) + rem
    if new_position > 122:
        new_position = 97 + (new_position - 122)
    encoded_text += chr(new_position)
return encoded_text

def decode(text, rotations, rem):
'''
Decodes the passed text, with the passed rotations 
'''
decoded_text = ""
for character in text:
    new_position = ord(character) - rem
    if new_position < 97:
        diff = 97 - new_position
        new_position = 122 - diff
    decoded_text += chr(new_position)
return decoded_text

And you call them up in main function like this:

if __name__ == '__main__':

rem = (rotations % 26)
ROTATIONS = random.randint(3, 26)
ENCODED = encode(random.choice(WORDS), ROTATIONS, rem)
DECODED = decode(ENCODED, ROTATIONS, rem)

Happy Coding :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.