The Replace Conditional with Polymorphism refactoring is most effective when you see the same conditional scattered throughout your code. When you need to add a new type of behaviour, you have to find and change every conditional to accommodate the new option. Instead, we concentrate the conditional logic in one place - the code that creates the polymorphic object - and just let the semantics of OO take care of the rest for us.
Here's a more egregious, straw-man form of your logging example.
if log_type == "file":
log_file.write("DEBUG: beginning script")
elif log_type == "database":
cursor.execute("INSERT INTO log_table (Level, Msg) VALUES ('DEBUG', 'beginning script')")
try:
file = open("/path/to/file")
lines = file.readlines()
if log_type == "file":
log_file.write("INFO: read {} lines".format(len(lines)))
elif log_type == "database":
cursor.execute("INSERT INTO log_table (Level, Msg) VALUES ('INFO', 'read {} lines')".format(len(lines)))
except:
if log_type == "file":
log_file.write("ERROR: failed to read file")
elif log_type == "database":
cursor.execute("INSERT INTO log_table (Level, Msg) VALUES ('ERROR', 'failed to read file')")
raise
finally:
if log_type == "file":
log_file.write("INFO: closing file")
elif log_type == "database":
cursor.execute("INSERT INTO log_table (Level, Msg) VALUES ('INFO', 'closing file')")
file.close()
You can see that the conditional logic examining the log type is executed three times, subtly differently each time. If we needed to add a new type of logging, like logging errors by email, we'd have to go through the whole script and add another elif
to every log statement, which is error-prone and cumbersome.
It's also quite hard to see at a glance what the script is actually doing, because it's swamped in the details of actually doing the logging.
So this is a great candidate for Replace Conditional With Polymorphism. Here are the logger classes after refactoring:
class AbstractLogger:
def debug(self, msg):
self.log("DEBUG", msg)
def info(self, msg):
self.log("INFO", msg)
def error(self, msg):
self.log("ERROR", msg)
def log(self, level, msg):
raise NotImplementedError()
class FileLogger(AbstractLogger):
def __init__(self, file):
self.file = file
def log(self, level, msg):
self.file.write("{}: {}".format(level, msg))
class DatabaseLogger(AbstractLogger):
def __init__(self, cursor):
self.cursor = cursor
def log(self, level, msg):
self.cursor.execute("INSERT INTO log_table (Level, Msg) VALUES ('{}', '{}')".format(level, msg))
I've used inheritance to avoid repeating too much code between the FileLogger and DatabaseLogger classes.
Here's the script:
# create the logger once at the start
if log_type == "file":
logger = FileLogger(log_file)
elif log_type == "database":
logger = DatabaseLogger(cursor)
logger.debug("beginning script")
try:
file = open("/path/to/file")
lines = file.readlines()
logger.info("read {} lines".format(len(lines)))
except:
logger.error("failed to read file")
raise
finally:
logger.info("closing file")
file.close()
It's now much easier to add a new type of logging: just write an EmailLogger
and modify the single conditional which creates it. The code is also much cleaner: the logger classes hide all the details of how they work behind a simple set of methods with logging-oriented names.