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())

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__.