# Caesar Cipher improvement

Currently, I have this and it's working for all cases:

import math
import os
import random
import re
import sys

def caesarCipher(s, k):
st = []
for i in range(0,len(s)):
if 65<=ord(s[i])<=90:
temp = ord(s[i])+k%26
if (temp>90):
temp-=26
st.append(chr(temp))
elif 97<=ord(s[i])<=122:
temp = ord(s[i])+k%26
if (temp>122):
temp-=26
st.append(chr(temp))
else: st.append(s[i])
return ''.join(st)

if __name__ == '__main__':

s = input()

k = int(input())

result = caesarCipher(s, k)


I'm not sure if the multiple if loops reduce readability. Is there a cleaner way to do this?

Sample I/O:

Input: s = middle-Outz , k=2
Output: s = okffng-Qwvb

Input: s = Always-Look-on-the-Bright-Side-of-Life ; k=5
Output: s = Fqbfdx-Qttp-ts-ymj-Gwnlmy-Xnij-tk-Qnkj


# Review

• Remove unused imports

You don't use any of these imports, just remove them to reduce clutter

• Read PEP8 the python style guide

1. Functions and variables should be snake_case
2. There should be a <space> in between operators
• Instead of appending to a list, you could create the string for the start

So instead of making st = [], do st = '' and st += char, no need to join then

However you could make the shifting a separate function, and then ''.join() becomes needed again

• When looping, loop over the item not the index

Instead of for i in range(len(s)) you could directly loop over the char --> for i in s

• There is a function to determine if we encounter an uppercase or lowercase

You could use char.isupper() or char.islower()

• Simplify the shifting

You could use the fact that if you subtract the start ord('a') or ord('A') and then modulo, the extra check of if temp > x becomes not needed

• Notice how the operations for each case (upper, lower) are mostly similar

# Putting it all together

import string
APLHA_LEN = len(string.ascii_lowercase)

def shift_letter(letter, shift):
start = ''
if letter.isupper():
start = ord('A')
elif letter.islower():
start = ord('a')
return chr((ord(letter) - start + k) % APLHA_LEN + start) if start else letter

def ceasar_cipher(st, shift):
return ''.join(shift_letter(letter, shift) for letter in st)


BUT, there is a better way...

As mentioned by someone already you could use the maketrans function very nicely for ceasar ciphers.

Giving you something like this

from string import ascii_lowercase as lc, ascii_uppercase as uc

def ceasar_cipher_translate(st, k):
trans = str.maketrans(lc + uc, lc[k:] + lc[:k] + uc[k:] + uc[:k])
return str.translate(st, trans)


NOTE That this code assumes Python3.x

• A magic number 26 is still lingering. Consider something along the lines of len(string.ascii_lowercase). – vnp Sep 18 '18 at 20:11
• @vnp Done, that slipped my mind :) – Ludisposed Sep 19 '18 at 13:00