Python conditionals replacement with polymorphism

Question

I've recently read an article/code snippet that shows an example of replacing conditionals with polymorphism. Here is the code:

Before:

def log_msg(log_type):
    msg = 'Operation successful'
    if  log_type == 'file':
        log_file.write(msg)
    elif log_type == 'database':
        cursor.execute('INSERT INTO log_table (MSG) VALUES ('?')', msg)

After:

class FileLogger(object):
    def log(self, msg):
        log_file.write(msg)

class DbLogger(object):
    def log(self, msg):
        cursor.execute('INSERT INTO log_table (MSG) VALUES ('?')', msg)

def log_msg(obj):
    msg = 'Operation successful'
    obj.log(msg)

Here is where I got it from.

Now my question is, in which significant way is the second method better than the first? For as far I know, if I want to use the second method, I have to do something like this every time I want to log something:

if log_type == 'file':
    log_msg(FileLogger())
elif: log_type == 'database':
    log_msg(DbLogger())

Am I missing the point or something very obvious?

Answer 1

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.

Answer 2

The point is that you would generally have created only one logger object at some earlier point in your program. So then you would just do log_msg(myLogger), and it would automatically do the right thing, whether you had originally decided to use file-based or db-based logging.

In other words, your code would look like this

# beginning of file
from logmodule import FileLogger, DBLogger, log_msg
myLogger = FileLogger()

# lots of other code here. . .
# later if you want to log something:

log_msg(myLogger)

Later, you could go back and change the beginning to myLogger = DBLogger() and everything would still work. The idea is that create the logger at the beginning of the progrma, and once you create it you don't need to worry about which kind you created, you can use it just the same.

Note that this example (including the code you originally posted) is just a skeleton; it's not code you could actually use directly. For one thing, this code doesn't provide any way to specify the log file name. What I'm describing here is just the idea of why you would restructure code like this.

Answer 3

I think what you are really searching for is something like this, you can avoid many if clauses for and while instantiation with a dictionary and copy and a proper set() method. I admit, that this has not much to do with polymophism, but will solve your problem of if-clauses for instantiation.

import copy
class Father:
    def __init__(self):
        self.string = "Wargs"
        
    def printString(self):
        print(self.string)

class A(Father):
    def __init__(self):
        pass
        
    def set(self, anything):
        self.string = "A:"+anything
        
        
class B(Father):
    def __init__(self):
        pass
        
    def set(self, anything):
        self.string = "B:"+anything 

class_dict = {"A": A(),
              "B": B()}
              
A_copy = copy.deepcopy(class_dict["A"])
A_copy.set("Happy")
A_copy.printString()

#Also possible:
class_dict2 = {"A": A,
              "B": B}
A_cl = class_dict2["A"]()
A_cl.set("Happy")
A_cl.printString()

I'm not sure about this, but I think such behaviour can also be reached with function-decorators, then you would overcome the hash-table access during runtime, what is even faster.