Implementation of Ceaser's Cipher in Python

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 helpe

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

StackExchange Code Review Q#145523, answer score: 3