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

Helper function to find rotation of Caesar Cipher

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

Problem

Just finished an assignment in Python where one part was to write a function to find the correct rotation in a encryption similar to a Caesar Cipher..

As mentioned it's already finished and works as intended. The course is an introduction to Python, but I do have experience with other languages since previously.

I was hoping someone could have a look at my findRot to see where I possibly could have done anything that isn't 'Python-esque' or what potential bottlenecks might occur.

def findRot(word1, word2, iterator):
    letterFreq = [("A", 65), ("E", 69), ("I", 73), ("O", 79), ("T", 84), ("U", 85)]
    rot = ord(word1) - letterFreq[iterator][1]
    letters = [chr(ord(letter) - rot) if (ord(letter) - rot >= 65) else chr(90 - (65 -(ord(word2[2]) - rot + 1))) for letter in word2]
    with open('words.txt', 'r') as words:
        lines = words.readlines()
        lines = list(map(lambda s: s.upper().strip(), lines))
        if ''.join(letters) in lines:
            print(''.join(letters) + ", " + str(True))
            return rot
        else:
            findRot(word1, word2, iterator + 1)


The assignment specified the usage of letter frequency, so I just took some of the more common letters found in English and put them in a list of tuples for readability purposes.

In the professor's use-case the first word would be 'R' and the second would be 'Uxen'. So in this case the rotation would supply the 'I' and 'Love'.

Again, this function just runs a primary check to find the rotation to apply to the rest of the 'encrypted' text. So a validation of the decrypted text will take place later.

EDIT:

Example usage:

>>> findRot('R', 'UXEN', 0)
9


Because it takes 9 rotations to get to the clear text I and Love.

Solution

First off, you should follow PEP8.
This will make your code more readable for Python programmers, and there is quite a few tips in there to make your code more Pythonic.
The amount of whitespace that you use is actually pretty good! It's mostly your names that 'let you down'.
But you're following a convention of camelCase. Which is also ok, but snake_case is preferred.

I don't like the re-definition of letterFreq and lines on ever function call.
This leads to a performance hindrance.
And so I'd move them out of the function to make them global.
Also as they are constants you may want to follow PEP8 and name them LETTER_FREQ and WORDS.

After this I'd look into removing the recursion. In Python recursion is cute, and un-reliable, nothing more.
I'd recommend that you don't use it, and instead you can use a for loop.
Using range starting at iterator and ending at len(LETTER_FREQ) we can remove the need for the recursion.

I'd also remove the print, instead of calling it in the function you should do it outside of the function.
If I'm using this function I don't want a print there ever couple of minutes.

Finally your turnery is a bit awkward and so I'd change it to use the modulo function, %.
This allows you to remove the if/else as -1 % 26 == 25 and so we can replace it with chr((ord(letter) - rot - 65) % 26 + 65).

Adding all the above together lead me to:

LETTER_FREQ = "AEIOTU"

with open('words.txt', 'r') as f:
    WORDS = [word.upper().strip() for word in f.readlines()]

def find_rotation(word1, word2, iterator=0):
    for index in range(iterator, len(LETTER_FREQ)):
        rot = ord(word1) - ord(LETTER_FREQ[index])
        letters = ''.join([chr((ord(letter) - rot - 65) % 26 + 65) for letter in word2])
        if letters in WORDS:
            return rot
    return None


For an additional speed you can also change WORDS to be a set, WORDS = set(WORDS), this is as it'll then be \$O(1)\$ look up in the if letters in WORDS: line, rather than \$O(n)\$. This is good as if the words list is large then it'll take a significantly larger amount of time then it would with a set.

Code Snippets

LETTER_FREQ = "AEIOTU"

with open('words.txt', 'r') as f:
    WORDS = [word.upper().strip() for word in f.readlines()]

def find_rotation(word1, word2, iterator=0):
    for index in range(iterator, len(LETTER_FREQ)):
        rot = ord(word1) - ord(LETTER_FREQ[index])
        letters = ''.join([chr((ord(letter) - rot - 65) % 26 + 65) for letter in word2])
        if letters in WORDS:
            return rot
    return None

Context

StackExchange Code Review Q#141550, answer score: 3

Revisions (0)

No revisions yet.