CODEWITHSUNDEEP
python
carlmonday
carlmonday

Reputation: 261

Yet another caesar cipher in python

I'm writing a function to shift text by 13 spaces. The converted chars need to preserve case, and if the characters aren't letters then they should pass through unshifted. I wrote the following function:

def rot13(str):
    result = ""
    for c in str:
        if 65 <= ord(c) <= 96:
            result += chr((ord(c) - ord('A') + 13)%26 + ord('A'))
        if 97 <= ord(c) <= 122:
            result += chr((ord(c) - ord('a') + 13)%26 + ord('a'))
        else:
            result += c
    print result

What I have found is that lowercase letters and non-letter characters work fine. However, when the function is applied to uppercase chars the function returns the shifted char FOLLOWED BY the original char. I know there are plenty of solutions to this problem on SO, but this specific error has me wondering what's wrong with my logic or understanding of chars and loops in python. Any help appreciated.

Upvotes: 2

Views: 166

Answers (2)

Rodrigo Rodrigues
Rodrigo Rodrigues

Reputation: 8556

This is an (rather contrived) one-liner implementation for the caesar cipher in python:

cipher = lambda w, s: ''.join(chr(a + (i - a + s) % 26) if (a := 65) <= (i := ord(c)) <= 90 or (a := 97) <= i <= 122 else c for c in w)

word = 'Hello, beautiful World!'
print(cipher(word, 13)) # positive shift
# Uryyb, ornhgvshy Jbeyq!

word = 'Uryyb, ornhgvshy Jbeyq!'
print(cipher(word, -13)) # -13 shift means decipher a +13 shift
# Hello, beautiful World!

It rotates only ascii alpha letters, respects upper/lower case, and handles positive and negative shifts (even shifts bigger that the alphabet).

More details in this answer.

Upvotes: 0

lejlot
lejlot

Reputation: 66805

You are missing the "else" statement, so if the first if "fires" (c is an uppercase letter) then the "else" from the second if also "fires" (and concatenates the uppercase letter, as ord(c) is not between 97 and 122)

def rot13(str):
    result = ""
    for c in str:
        if 65 <= ord(c) <= 96:
            result += chr((ord(c) - ord('A') + 13)%26 + ord('A'))
        elif 97 <= ord(c) <= 122:
            result += chr((ord(c) - ord('a') + 13)%26 + ord('a'))
        else:
            result += c
    print result

Also, uppercase characters end with ord('Z')==90, ASCII characters between 91 and 96 are not letters. Function should also return the value, not print it (unless it is called print_rot13). Your function is also inconsistent - you use ord('A') in calculations, but actual, hard-coded value in if (65) you should decide on one of these.

def rot13(str):
    a = ord('a')
    z = ord('z')
    A = ord('A')
    Z = ord('Z')
    result = ""
    for c in str:
        symbol = ord(c) 
        if A <= symbol <= Z:
            result += chr((symbol - A + 13)%26 + A)
        elif a <= symbol <= z:
            result += chr((symbol - a + 13)%26 + a)
        else:
            result += symbol
    return result

This way it only assume, that lower and upper case letters are arranged in consistent blocks, but nothing about their actual ord values.

Upvotes: 2

Related Questions