Caesar cipher brute force

Question

I have code that uses the brute force technique to find the message to a Caesar cipher. The program decrypts the message using different keys and then checks whether the resulting words are contained in a file called 'wordlist.txt'. if they are, it returns only that result.

from collections import namedtuple
Keycode = namedtuple('Keycode', 'key code')

def picks(filename):
    with open(filename) as f:
        lines = [line.strip() for line in f]
    return lines

message = 'vlr alkq exsb ql qefkh xylrq qexq exex.'
LETTERS = 'abcdefghijklmnopqrstuvwxyz'
result = []
for key in range(len(LETTERS)):
    translated = ''
    for symbol in message.lower():
        if symbol in LETTERS:
            num = LETTERS.find(symbol) 
            num = num - key
            if num < 0:
                num = num + len(LETTERS)
            translated += LETTERS[num]
        else:
            translated += symbol
    result.append(Keycode(key, translated))

for item in result:
    d = item.code.split()
    for word in d:
        if word in picks('wordlist.txt'):
            print('Code : {0:s}\nKey  : {1:}'.format(item.code, item.key))
            break

The code that I have does the job but it takes time and the code is not clean. Is there a way to make the code a bit cleaner? I am new to Python and haven't started on classes, so I cant use those (although it would have made my job easier).

Answer 1

Split your code into small units

If you split you split your code, (most of the time) you automatically make it easier to read and to test. Also, it allows you to use Python syntactic sugar like list comprehension much more easily.

Modulo operator

As suggested in the comments, the % operator allows you to write things in a smarter way.

Better than abcdef...z

You can use string.ascii_lowercase (after importing string) instead of defining it yourself.

Use join instead of string concatenations

As adviced by the Python Style Guide called PEP 8 :

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 is very straightforward once you have a function to encode a single letter.

At this stage, the code looks like :

def caesar_letter(symbol, key):
    if symbol in LETTERS:
        return LETTERS[(LETTERS.find(symbol) - key) % len(LETTERS)]
    else:
        return symbol

def caesar(msg, key):
    return ''.join(caesar_letter(l, key) for l in msg.lower())

result = [Keycode(key, caesar(message, key)) for key in range(len(LETTERS))]

Another tips

Instead of generating all possible strings only to stop once you've found the one you want, you could generate them at you go. If the first string is the one you want, you'll avoid most of the work.

for key in range(len(LETTERS)):
    code = caesar(message, key)
    d = code.split()
    for word in d:
        if word in picks('wordlist.txt'):
            print('Code : {0:s}\nKey  : {1:}'.format(code, key))
            break

if __name__ == "__main__":

You should move your code actually doing stuff behind an if __name__ == "__main__": guard. I'll let you read more about this.