Implementation of Ceaser's Cipher in Python

Question

This is my implementation of a Ceaser's Cipher in Python using OOP principles. I'm looking for feedback for any code starting from the function build_shift_dict (line 89) as I wrote that code myself. Feedback on the earlier parts will be useful but not as much as it's not my actual code (but is included so the whole program can work).

import string
import copy
def load_words(file_name):
 '''
 file_name (string): the name of the file containing 
 the list of words to load 
 Returns: a list of valid words. Words are strings of lowercase letters.
 Depending on the size of the word list, this function may
 take a while to finish.
 '''
 print('Loading word list from file...')
 # inFile: file
 in_file = open(file_name, 'r')
 # line: string
 line = in_file.readline()
 # word_list: list of strings
 word_list = line.split()
 print(' ', len(word_list), 'words loaded.')
 in_file.close()
 return word_list
def is_word(word_list, word):
 '''
 Determines if word is a valid word, ignoring
 capitalization and punctuation
 word_list (list): list of words in the dictionary.
 word (string): a possible word.
 Returns: True if word is in word_list, False otherwise
 Example:
 >>> is_word(word_list, 'bat') returns
 True
 >>> is_word(word_list, 'asdf') returns
 False
 '''
 word = word.lower()
 word = word.strip(" !@#$%^&*()-_+={}[]|\:;'<>?,./\"")
 return word in word_list
def get_story_string():
 """
 Returns: a joke in encrypted text.
 """
 f = open("story.txt", "r")
 story = str(f.read())
 f.close()
 return story
WORDLIST_FILENAME = 'words.txt'
class Message(object):
 def __init__(self, text):
 '''
 Initializes a Message object
 text (string): the message's text
 a Message object has two attributes:
 self.message_text (string, determined by input text)
 self.valid_words (list, determined using helper function load_words
 '''
 self.message_text = text
 self.valid_words = load_words(WORDLIST_FILENAME)
 def get_message_text(self):
 '''
 Used to safely access self.message_text outside of the class
 Returns: self.message_text
 '''
 return self.message_text
 def get_valid_words(self):
 '''
 Used to safely access a copy of self.valid_words outside of the class
 Returns: a COPY of self.valid_words
 '''
 return self.valid_words[:]
 def build_shift_dict(self, shift):
 '''
 Creates a dictionary that can be used to apply a cipher to a letter.
 The dictionary maps every uppercase and lowercase letter to a
 character shifted down the alphabet by the input shift. The dictionary
 should have 52 keys of all the uppercase letters and all the lowercase
 letters only.
 shift (integer): the amount by which to shift every letter of the
 alphabet. 0 <= shift < 26
 Returns: a dictionary mapping a letter (string) to
 another letter (string).
 '''
 lower_letters_to_values = {
 'a': 1, 'b': 2, 'c': 3, 'd': 4,
 'e': 5, 'f': 6, 'g': 7, 'h': 8,
 'i': 9, 'j': 10, 'k': 11, 'l': 12,
 'm': 13, 'n': 14, 'o': 15, 'p': 16,
 'q': 17, 'r': 18, 's': 19, 't': 20,
 'u': 21, 'v': 22, 'w': 23, 'x': 24,
 'y': 25, 'z': 26
 }
 lower_values_to_letters = {
 1: 'a', 2: 'b', 3: 'c', 4: 'd',
 5: 'e', 6: 'f', 7: 'g', 8: 'h',
 9: 'i', 10: 'j', 11: 'k', 12: 'l',
 13: 'm', 14: 'n', 15: 'o', 16: 'p',
 17: 'q', 18: 'r', 19: 's', 20: 't',
 21: 'u', 22: 'v', 23: 'w', 24: 'x',
 25: 'y', 26: 'z'
 }
 upper_letters_to_values = {
 'A': 1, 'B': 2, 'C': 3, 'D': 4,
 'E': 5, 'F': 6, 'G': 7, 'H': 8,
 'I': 9, 'J': 10, 'K': 11, 'L': 12,
 'M': 13, 'N': 14, 'O': 15, 'P': 16,
 'Q': 17, 'R': 18, 'S': 19, 'T': 20,
 'U': 21, 'V': 22, 'W': 23, 'X': 24,
 'Y': 25, 'Z': 26
 }
 upper_values_to_letters = {
 1: 'A', 2: 'B', 3: 'C', 4: 'D',
 5: 'E', 6: 'F', 7: 'G', 8: 'H',
 9: 'I', 10: 'J', 11: 'K', 12: 'L',
 13: 'M', 14: 'N', 15: 'O', 16: 'P',
 17: 'Q', 18: 'R', 19: 'S', 20: 'T',
 21: 'U', 22: 'V', 23: 'W', 24: 'X',
 25: 'Y', 26: 'Z'
 }
 def shift_dict(dictionary, shift):
 dict = {}
 # The key should be a letter and the value should be a number
 for key in dictionary:
 # checking that when i add the shift to the value the value doesn't exceed the size of the alphabet.
 if dictionary[key] <= 26 - shift:
 dict[key] = (dictionary[key] + shift)
 # if it does exceed the size i subtract the size of the alphabet to start counting from beginning
 else:
 dict[key] = (dictionary[key] + shift - 26)
 return dict
 # function switches the keys and values of a dictionary
 def inv_dict(dictionary):
 dictmap = {}
 for key in dictionary:
 dictmap[dictionary[key]] = key
 return dictmap
 # since the keys should now be number's mapped to a new letter, i replace the number with its original letter
 def change_keys_to_letters(dictionary, mapping):
 dict = {}
 for key in dictionary:
 dict[mapping[key]] = dictionary[key]
 return dict
 def put_it_all_together(dictionary, shift, reference_table):
 local_dictionary = shift_dict(dictionary, shift)
 local_dictionary = inv_dict(local_dictionary)
 local_dictionary = change_keys_to_letters(local_dictionary, reference_table)
 local_dictionary = inv_dict(local_dictionary)
 return local_dictionary
 shifted_lower = put_it_all_together(lower_letters_to_values, shift, lower_values_to_letters)
 shifted_upper = put_it_all_together(upper_letters_to_values, shift, upper_values_to_letters)
 return {**shifted_lower, **shifted_upper}
 def apply_shift(self, shift):
 '''
 Applies the Caesar Cipher to self.message_text with the input shift.
 Creates a new string that is self.message_text shifted down the
 alphabet by some number of characters determined by the input shift 
 shift (integer): the shift with which to encrypt the message.
 0 <= shift < 26
 Returns: the message text (string) in which every character is shifted
 down the alphabet by the input shift
 '''
 dictionary = self.build_shift_dict(shift)
 text = self.get_message_text()
 shift_text = ''
 for char in text:
 try:
 shift_text = shift_text + dictionary[char]
 except KeyError:
 shift_text = shift_text + char
 return shift_text
class PlaintextMessage(Message):
 def __init__(self, text, shift):
 '''
 Initializes a PlaintextMessage object 
 text (string): the message's text
 shift (integer): the shift associated with this message
 A PlaintextMessage object inherits from Message and has five attributes:
 self.message_text (string, determined by input text)
 self.valid_words (list, determined using helper function load_words)
 self.shift (integer, determined by input shift)
 self.encrypting_dict (dictionary, built using shift)
 self.message_text_encrypted (string, created using shift)
 Hint: consider using the parent class constructor so less 
 code is repeated
 '''
 Message.__init__(self, text)
 self.shift = shift
 self.encrypting_dict = self.build_shift_dict(shift)
 self.message_text_encrypted = self.apply_shift(shift)
 def get_shift(self):
 '''
 Used to safely access self.shift outside of the class
 Returns: self.shift
 '''
 return self.shift
 def get_encrypting_dict(self):
 '''
 Used to safely access a copy self.encrypting_dict outside of the class
 Returns: a COPY of self.encrypting_dict
 '''
 copy = copy.deepcopy(self.encrypting_dict)
 return copy
 def get_message_text_encrypted(self):
 '''
 Used to safely access self.message_text_encrypted outside of the class
 Returns: self.message_text_encrypted
 '''
 return self.message_text_encrypted
 def change_shift(self, shift):
 '''
 Changes self.shift of the PlaintextMessage and updates other 
 attributes determined by shift (ie. self.encrypting_dict and 
 message_text_encrypted).
 shift (integer): the new shift that should be associated with this message.
 0 <= shift < 26
 Returns: nothing
 '''
 text = self.get_message_text(self)
 self.__init__(text, shift)
 return None
class CiphertextMessage(Message):
 def __init__(self, text):
 '''
 Initializes a CiphertextMessage object
 text (string): the message's text
 a CiphertextMessage object has two attributes:
 self.message_text (string, determined by input text)
 self.valid_words (list, determined using helper function load_words)
 '''
 Message.__init__(self, text)
 def decrypt_message(self):
 '''
 Decrypt self.message_text by trying every possible shift value
 and find the "best" one. We will define "best" as the shift that
 creates the maximum number of real words when we use apply_shift(shift)
 on the message text. If s is the original shift value used to encrypt
 the message, then we would expect 26 - s to be the best shift value
 for decrypting it.
 Note: if multiple shifts are equally good such that they all create
 the maximum number of you may choose any of those shifts (and their
 corresponding decrypted messages) to return
 Returns: a tuple of the best shift value used to decrypt the message
 and the decrypted message text using that shift value
 '''
 final_shift = 0
 best_word_list = 0
 word_list = load_words('words.txt')
 for shift in range(27):
 decrypted_text = self.apply_shift(shift)
 wordlist = decrypted_text.split(' ')
 num_valid_words = 0
 for word in wordlist:
 if is_word(word_list, word):
 num_valid_words += 1
 if num_valid_words > best_word_list:
 final_shift = shift
 best_word_list = num_valid_words
 return (final_shift, self.apply_shift(final_shift))
#Example test case (PlaintextMessage)
plaintext = PlaintextMessage('hello', 2)
print('Expected Output: jgnnq')
print('Actual Output:', plaintext.get_message_text_encrypted())
#Example test case (CiphertextMessage)
ciphertext = CiphertextMessage('jgnnq')
print('Expected Output:', (24, 'hello'))
print('Actual Output:', ciphertext.decrypt_message())

• ChrisIkeokwu: I didn't feel comfortable posting such a long code sample, is there a better way of doing this? i.e. Posting such a long sample for review.
• ferada: That's totally fine. There are often posts here with way longer code, don't worry about it.

Answer 1

Well, that is a pretty huge review. So I will point only major issues here. 1. Instead of using open as function use it as contextmanager:

def get_story_string():
 """
 Returns: a joke in encrypted text.
 """
 f = open("story.txt", "r")
 story = str(f.read())
 f.close()
 return story

Will be,

def get_story_string():
 """
 Returns: a joke in encrypted text.
 """
 with open("story.txt", "r") as f:
 return f.read()

But I wouldn't even create a function that is used only once and all it does is just returns a content of a file.

Function load_words should return a set instead of a list, since you do lots of "in" operations with it. It will increase lookup speed from O(n) to O(1)

so it will look like this:

def load_words(file_name):
 """
 file_name (string): the name of the file containing 
 the list of words to load 
 Returns: a list of valid words. Words are strings of lowercase letters.
 Depending on the size of the word list, this function may
 take a while to finish.
 """
 print('Loading word list from file...')
 # inFile: file
 with open(file_name, 'r') as in_file:
 word_list = set(in_file.readline().split())
 print(' ', len(word_list), 'words loaded.')
 return word_list

Please, note that docstrings should use triple double quotes. Not triple single as you do.

def get_message_text(self):
 '''
 Used to safely access self.message_text outside of the class
 Returns: self.message_text
 '''
 return self.message_text

Now here if you want to protect your message_test, you should also think about making it private. So self.message_text should be self._message_text Also if you want to make it pretty you can think about using property and define only setter.

Method build_shift_dict is a way to complicated. Let's try to make it easier. This dict lower_values_to_letters can be replaced with just 2 lines:

import string
lower_values_to_letters = dict(enumirate(1, string.ascii_lower))

however all this method can be replaced with 2 small ones:

def _get_shifted_iterator(shift, iterable):
 it = cycle(iterable)
 # skip first N characters
 for _ in range(shift):
 next(it)
 for element in it:
 yield element
def build_shift_dict(self, shift):
 return dict(chain(zip(string.ascii_lowercase, self._get_shifted_iterator(shift, string.ascii_lowercase)),
 zip(string.ascii_uppercase, self._get_shifted_iterator(shift, string.ascii_uppercase))))

But don't forget to import cycle and chain from itertools

Speaking about apply_shift, if you want to construct string it is better to use .join it is prettier, better in terms of memory usage and a pythonic way to do that.

def apply_shift(self, shift):
 dictionary = self.build_shift_dict(shift)
 text = self.get_message_text()
 shift_chars = []
 for char in text:
 try:
 shift_chars.append(dictionary[char])
 except KeyError:
 shift_chars.append(char)
 return ''.join(shift_chars)

Now we can go to PlaintextMessage, if you want to call parent method you should use function super so instead of this:

Message.__init__(self, text)

Just do:

super().__init__(self, text)

In your change_shift, you don't have to return None explicitly, if a function does not return anything then it returns None. There are no procedures in Python, they all are functions. Also, I would not call __init__ for self inside a class. If you want to change a state of the object, make your own method for that. It just seems to be not right to call __init__.

• ChrisIkeokwu: Thank you very much!! Will implement these ideas in any code i write