HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Caesar cipher exercise

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
caesarexercisecipher

Problem

I was doing this exercise from here. I made a solution work but can't help feel it could have been done in fewer lines. I'm quite new to python so I'm not too sure of the little shortcuts.

def shift(S, n):
   word = ''
   for i in S:
      if ord(i) == 32:
         word += ' '
      elif ord(i)+n > 90:
         a = n-(90-ord(i))+64
         word += chr(a)
      else:
         word +=chr(ord(i)+n)
   return word
def TGS(x): # Total goodness of string
   TGC = 0 # Total goodness of character
   for i in x:
      if ord(i) == 32:
         continue
      TGC += letterGoodness[ord(i)-65]
   return TGC
A = input()
LOSS = [shift(A,n) for n in range (1,27)] # List of shifted strings
LOTG = [TGS(x) for x in LOSS]
a = LOTG.index(max(LOTG))
print(LOSS[a])

Solution

-
Magic numbers.

Try to avoid them. 65 is really ord('A'), 90 is ord('Z'); so say that explicitly. Along the same line,

if i == ' ':


looks better than

if ord(i) == 32:


-
Naming

Try to use descriptive names. I'd prefer caesar_decrypt(ciphertext, shift) to shift(S, n). Same goes for A, a, etc.

-
Streamlining

I was really stumbled upon an asymmetry of elif and else cases of shift:

a = n-(90-ord(i))+64
     word += chr(a)


is quite counter-intuitive. I'd suggest

for i in S:
  if ord(i) == 32:
      word += ' '
      continue
  a = ord(i) + n
  if a > ord('Z'):
      a -= ord('Z') - ord('A')
  word +=chr(a)

Code Snippets

if i == ' ':
if ord(i) == 32:
a = n-(90-ord(i))+64
     word += chr(a)
for i in S:
  if ord(i) == 32:
      word += ' '
      continue
  a = ord(i) + n
  if a > ord('Z'):
      a -= ord('Z') - ord('A')
  word +=chr(a)

Context

StackExchange Code Review Q#61503, answer score: 4

Revisions (0)

No revisions yet.