Decode Cæsar Cypher by Checking all Keyspaces

This function produces a decoded string, decoded, for all keyspaces against a given cæsar cipher ciphertext. Punctuation is not changed and string.upper() is used to make working with ord() easier.

I am looking for a more Pythonic way to work with my decoded variable. I keep finding myself assigning empty strings or None or 0 to use as a placeholder and I think it looks ugly. I'd also like to know if there is a better way to avoid writing c = ord(c).

def testCaesar(ciphertext):
    for testkey in range(0,25):
        decoded = ''   # <--- uglyness
        for c in ciphertext.upper():
            if not c.isalpha():
                decoded += c
                continue
            c = ord(c) # this is here to improve readability of line below. Is there a better way?
            decoded += chr(c + testkey if c + testkey < 90 else c - testkey)
        print('{0} : Rot{1} Caesar').format(decoded, str(testkey))
        checkMatch(decoded, testkey, 'Caesar')

You could use else instead of continue to mark the 2 potential results of your test. You get something like :

def testCaesar(ciphertext):
    for testkey in range(0,25):
        decoded = ''   # <--- uglyness
        for c in ciphertext.upper():
            if not c.isalpha():
                decoded += c
            else:
                c = ord(c) # this is here to improve readability of line below. Is there a better way?
                decoded += chr(c + testkey if c + testkey < 90 else c - testkey)
        print('{0} : Rot{1} Caesar').format(decoded, str(testkey))
        checkMatch(decoded, testkey, 'Caesar')

Now, you've got some duplicated logic you could try to remove. You could use a new variable like :

def testCaesar(ciphertext):
    for testkey in range(0,25):
        decoded = ''   # <--- uglyness
        for c in ciphertext.upper():
            if not c.isalpha():
                decoded_c = c
            else:
                c = ord(c) # this is here to improve readability of line below. Is there a better way?
                decoded_c = chr(c + testkey if c + testkey < 90 else c - testkey)
            decoded += decoded_c
        print('{0} : Rot{1} Caesar').format(decoded, str(testkey))
        checkMatch(decoded, testkey, 'Caesar')

Or you could define a function for this :

def decode_char(c, key):
    if not c.isalpha():
        return c
    else:
        c = ord(c) # this is here to improve readability of line below. Is there a better way?
        return chr(c + testkey if c + testkey < 90 else c - testkey)

def testCaesar(ciphertext):
    for testkey in range(0,25):
        decoded = ''   # <--- uglyness
        for c in ciphertext.upper():
            decoded += decode_char(c, testkey)
        print('{0} : Rot{1} Caesar').format(decoded, str(testkey))
        checkMatch(decoded, testkey, 'Caesar')

Now, Python has a style guide called PEP8 and it says :


Do not rely on CPython's efficient implementation of in-place string
concatenation for statements in the form a += b or a = a + b . This
optimization is fragile even in CPython (it only works for some types)
and isn't present at all in implementations that don't use
refcounting. In performance sensitive parts of the library, the
''.join() form should be used instead. This will ensure that
concatenation occurs in linear time across various implementations.

This can be easily done now :

decoded = ''.join(decode_char(c, testkey) for c in ciphertext.upper())

Also, a tiny optimisation could be to call upper() once and only once.

StackExchange Code Review Q#118505, answer score: 4