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?
Benjamin Hodgson :
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.\n\n\n\nHere's a more egregious, straw-man form of your logging example.\n\nif log_type == \"file\":\n log_file.write(\"DEBUG: beginning script\")\nelif log_type == \"database\":\n cursor.execute(\"INSERT INTO log_table (Level, Msg) VALUES ('DEBUG', 'beginning script')\")\n\ntry:\n file = open(\"/path/to/file\")\n lines = file.readlines()\n\n if log_type == \"file\":\n log_file.write(\"INFO: read {} lines\".format(len(lines)))\n elif log_type == \"database\":\n cursor.execute(\"INSERT INTO log_table (Level, Msg) VALUES ('INFO', 'read {} lines')\".format(len(lines)))\nexcept:\n if log_type == \"file\":\n log_file.write(\"ERROR: failed to read file\")\n elif log_type == \"database\":\n cursor.execute(\"INSERT INTO log_table (Level, Msg) VALUES ('ERROR', 'failed to read file')\")\n\n raise\nfinally:\n if log_type == \"file\":\n log_file.write(\"INFO: closing file\")\n elif log_type == \"database\":\n cursor.execute(\"INSERT INTO log_table (Level, Msg) VALUES ('INFO', 'closing file')\")\n file.close()\n\n\nYou 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.\n\nIt'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.\n\n\n\nSo this is a great candidate for Replace Conditional With Polymorphism. Here are the logger classes after refactoring:\n\nclass AbstractLogger:\n def debug(self, msg):\n self.log(\"DEBUG\", msg)\n\n def info(self, msg):\n self.log(\"INFO\", msg)\n\n def error(self, msg):\n self.log(\"ERROR\", msg)\n\n def log(self, level, msg):\n raise NotImplementedError()\n\nclass FileLogger(AbstractLogger):\n def __init__(self, file):\n self.file = file\n\n def log(self, level, msg):\n self.file.write(\"{}: {}\".format(level, msg))\n\nclass DatabaseLogger(AbstractLogger):\n def __init__(self, cursor):\n self.cursor = cursor\n\n def log(self, level, msg):\n self.cursor.execute(\"INSERT INTO log_table (Level, Msg) VALUES ('{}', '{}')\".format(level, msg))\n\n\nI've used inheritance to avoid repeating too much code between the FileLogger and DatabaseLogger classes.\n\nHere's the script:\n\n# create the logger once at the start\nif log_type == \"file\":\n logger = FileLogger(log_file)\nelif log_type == \"database\":\n logger = DatabaseLogger(cursor)\n\nlogger.debug(\"beginning script\")\n\ntry:\n file = open(\"/path/to/file\")\n lines = file.readlines()\n\n logger.info(\"read {} lines\".format(len(lines)))\nexcept:\n logger.error(\"failed to read file\")\n raise\nfinally:\n logger.info(\"closing file\")\n file.close()\n\n\nIt'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.",
2014-07-01T19:19:51
BrenBarn :
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.\n\nIn other words, your code would look like this\n\n# beginning of file\nfrom logmodule import FileLogger, DBLogger, log_msg\nmyLogger = FileLogger()\n\n# lots of other code here. . .\n# later if you want to log something:\n\nlog_msg(myLogger)\n\n\nLater, 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.\n\nNote 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.",
2014-07-01T18:33:57