- blah blah
- blah blah
Last active
October 24, 2023 03:01
-
-
Save jbrucker/0103ec3212d8f0b9e9012c6d45ffa9df to your computer and use it in GitHub Desktop.
Example MovieCatalog code to refactor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
from movie import Movie | |
class MovieCatalog: | |
"""A movie catalog.""" # 7. Communicate in Code | |
def __init__(self): | |
self.catalog = [] # 6. Divide Long Method | |
with open('movies.csv', 'r') as file: # 1. Rename | |
rows = csv.reader(file) # 1. Rename, 5. Fix Latent Error | |
for r in rows: # 1. Rename | |
if not r: | |
continue | |
if not r[0].startswith('#'): # 2. Replace Nested if with Guard | |
try: | |
self.catalog.append( | |
Movie(r[1],r[2],r[3].split('|'))) # 4. Intro Var | |
except IndexError: # 3. Replace Exception with Test | |
logging.error( | |
f'Line {rows.line_num}:\ | |
Unrecognized format "{",".join(r)}"') | |
def get_movie(self, movie_name, year: int = None): | |
"""Get a movie from the catalog that matches the parameter values.""" | |
all_match: Dict[int,Movie] = {} | |
for movie in self.catalog: | |
if year is not None: | |
if year == movie.year and movie.get_title() == movie_name: | |
return movie | |
else: | |
if movie.get_title() == movie_name: | |
all_match[movie.year] = movie | |
if all_match: | |
return all_match[max(all_match)] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# After Refactoring | |
import csv | |
import logging | |
from movie import Movie | |
from typing import Dict | |
MOVIE_DATA_FILE = 'movies.csv' | |
class MovieCatalog: | |
"""Searchable collection of Movies, using data from a file.""" | |
def __init__(self): | |
self.catalog = self.load_movie_catalog(MOVIE_DATA_FILE) | |
def load_movie_catalog(self, data_file): | |
"""Read CSV data from a file and create movies.""" | |
catalog = [] | |
with open(data_file, 'r') as csv_file: | |
csv_data = csv.reader(csv_file, skipinitialspace=True) | |
for movie_data in csv_data: | |
# the CSV data may contain blank lines and comment (#) lines | |
if not movie_data or movie_data[0].startswith('#'): | |
continue | |
if len(movie_data) != 4: | |
logging.error( | |
f'Line {csv_data.line_num}: Unrecognized format "{",".join(movie_data)}"') | |
continue | |
(_, title, year, genre_string) = movie_data | |
catalog.append(Movie(title, year, genre_string.split('|'))) | |
return catalog | |
def get_movie(self, title, year = None): | |
"""Get a movie from the catalog that matches the parameter values.""" | |
# 1. avoid repeated "if year is not None" inside of long for loop | |
# 2. replace for loop with list comprehension | |
# Movies are in reverse chronological order, so if the year | |
# is not specified then return the first match by title | |
if year is None: | |
# match by title only | |
matches = lambda movie : movie.title == title | |
else: | |
# match by title and year released | |
matches = lambda movie: movie.title == title and movie.year == year | |
try: | |
# use next() for short-circuit evaluation. Returns first match. | |
return next(movie for movie in self.catalog if matches(movie)) | |
except StopIteration: | |
# no match | |
return None |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment