patternpythonMinor
Helper function to find rotation of Caesar Cipher
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.
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:
Because it takes 9 rotations to get to the clear text
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)
9Because 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
I don't like the re-definition of
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
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
I'd also remove the
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
Adding all the above together lead me to:
For an additional speed you can also change
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 NoneFor 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 NoneContext
StackExchange Code Review Q#141550, answer score: 3
Revisions (0)
No revisions yet.