Python Caesar function wrong output

Viewed 69

I am following 100 days of code and I'm having trouble understanding why this function isn't working properly.

alphabet = ["a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","r","s","t","u","v","w","x","y","z",
            "a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","r","s","t","u","v","w","x","y","z",
            "a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","r","s","t","u","v","w","x","y","z"]

direction = input("Type 'encode' for Encode type 'decode' for Decode")
text = input("Type your message: \n").lower()
shift = int(input("Type the shift number: \n"))

def cesar(input_text,shift_ammount,which_direction):
    word = ""
    for letter in input_text:
        position = alphabet.index(letter)
        if which_direction == "decode":
            shift_ammount *= -1
        new_position = position + shift_ammount
        word += alphabet[new_position]
    print(f"the {which_direction}d text is {word}")

cesar(input_text=text,shift_ammount=shift,which_direction=direction)

Let's say I'm trying to decode the string "bcd". It returns "adc" instead of "abc". For some reason, it adds instead of subtracting to the second position and returns the wrong value. Any help would be welcomed!

3 Answers

The problem is the line:

shift_ammount *= -1

This line is inside the for-loop which means that the shift_ammount will change the value in each iteration. Place this part of the code outside the for-loop to get the correct result.

Updated code:

def cesar(input_text, shift_ammount, which_direction):
    word = ""
    if which_direction == "decode":
        shift_ammount *= -1
    for letter in input_text:
        position = alphabet.index(letter)
        new_position = position + shift_ammount
        word += alphabet[new_position]
    print(f"the {which_direction}d text is {word}")

I checked your code:

If I encode abcdef with shift 1 it gives me bcdefg - it works

If I decode bcdefg with shift 1 it gives me adcfeh - every second letter is broken. This is because the shift_amout *= -1 switches the direction every loop when decoding

My solution is to put the if decode ABOVE the for-loop, like

alphabet = ["a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","r","s","t","u","v","w","x","y","z",
            "a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","r","s","t","u","v","w","x","y","z",
            "a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","r","s","t","u","v","w","x","y","z"]

direction = input("Type 'encode' for Encode type 'decode' for Decode: \n") #not part of the question, but you forgot the /n here 
text = input("Type your message: \n").lower()
shift = int(input("Type the shift number: \n"))

def cesar(input_text,shift_ammount,which_direction):
    if which_direction == "decode":
        shift_ammount *= -1

    word = ""
    for letter in input_text:
        position = alphabet.index(letter)
        new_position = position + shift_ammount
        word += alphabet[new_position]
    print(f"the {which_direction}d text is {word}")

cesar(input_text=text,shift_ammount=shift,which_direction=direction)

The problem in your code has already been solved by others in the thread, so I would just like to say that the Cesar-cipher is a perfect example for where str.translate is the perfect solution for the problem.

Using str.maketrans in conjuction with an offset we can dynamically translate the string, using a couple of dict-comprehensions:

import string
def cesar(text: str, offset: int, decode=False) -> str:
    alpha = string.ascii_lowercase
    if decode:
        cipher = {let: alpha[(idx - offset) % len(alpha)] for idx, let in enumerate(alpha)}
    else:
        cipher = {let: alpha[(idx + offset) % len(alpha)] for idx, let in enumerate(alpha)}
    return text.translate(str.maketrans(cipher))

x = "this is a test-string!"
newx = cesar(x, 1)
print(x)
print(newx)
print(cesar(newx, 1, decode=True))

Output:

this is a test-string!
uijt jt b uftu-tusjoh!
this is a test-string!
Related