Refractoring OOP vs. Functional Programming Approach

I am writing a python script that will call mandrills export activity api for every blog on a worpress mu install except for the main site. The script is designed to be called from a cron job on a regular schedule. The code I have is here.

```
__author__ = 'Taylor'

import datetime
import mandrill
import phpserialize
import mysql.connector

class BlogReporter(object):
blog_id = None
table_name = 'wp_%s_options'
notify_email = None
sender_email = None
date_to = None
date_from = None

def __init__(self, blog_id_, database_config_, mandrill_client_):
self.blog_id = blog_id_
self.mandrill_client = mandrill_client_
self.table_name = 'wp_' + str(blog_id_) + '_options'
try:
cnx_ = mysql.connector.connect(**database_config_)
except mysql.connector.Error as err:
print(err)
else:
query_ = "SELECT option_value FROM " + self.table_name + " WHERE option_name=%s"
try:
cursor_ = cnx_.cursor()
cursor_.execute(query_, ('admin_email',))
result = cursor_.fetchone()
except Exception as err:
print(err)
else:
# set notify email from cursor
self.notify_email = result
cursor_.close()
try:
cursor_ = cnx_.cursor()
cursor_.execute(query_, ('wysija',))
encoded_result = cursor_.fetchone()[0]
except Exception as err:
print(err)
else:
decoded_result = phpserialize.unserialize(encoded_result.decode('base64', 'strict'))
self.sender_email = decoded_result['from_email']
cursor_.close()
cnx_.close()
self.date_to = datetime.datetime.now()
# if it monday lets get activity since saturday
if self.date_to.weekday() is 0:
self.date_from = self.date_to - datet

Here are some points:


and wanted to learn more about classes in python

If you write some piece of code to learn something, delete it and rewrite it.


construct[o]r and method export_activity could be refactored into one function

If you have a class with a single method converting to a function is what you should do by default. By by default I mean unless you have a really solid reason to do otherwise.


I was curious if doing so would be beneficial to the program in any way.

Simplicity is second only to (actually needed) functionality. (As XP people say Do the simplest thing that could possibly work.) You do not have to justify making your program simpler by appealing to a higher value.

Some more specific points:

The sections of code where you read an option from the database is the same almost word for word. If you find yourself writing the same code twice or more tellingly Copy&Pasteing some snippet you should put it in a function and call it instead. This way you will save yourself a lot of effort. As a general rule you should write everything once and only once.

Your constructor does too much. Anything other that assigning fields should not be done in a constructor. But your constructor is too long even for a method. Extract every snippet of code that is doing something in a function so that when you (or someone else) read it again instead of trying to guess what you are trying to do by reading how you do it you would just read what you are trying to do. This way you (or others) may skip reading the extracted function altogether if they are not interested.

Clean up of resources (files, connections, cursors etx) are usually done by initializing them in with blocks. If the resource does not support with blocks out-of-the box but cleaning-up consists of calling .close() on it, you can wrap the resource in contextlib .closing The clean-up should otherwise be done in finally blocks to ensure they are executed even if an exception is thrown; at least in long running programs, server or desktop applications.

When you decide to catch an exception, catch the most specific exception you expect. Look what else are you catching when you say catch Exception. Here root exception of all mysql.connector errors (apparently mysql.connector.Error) is more suitable.

If you are not going to do anything useful (recover, wrap&rethrow etc) do not catch exceptions. You can do print(err) somewhere else.

In a script put your code in a function, for example main, and call it if __name__ == "__main__": main(). That way you can debug your code in repl, or import it in other scripts/programs as a module. This also allows further refactorings, which I won't elaborate but you can see in the code below.

If a method has many parameters and you are calling it many times with same parameters you can use functools.partial to reduce the noise. (see below)

Also cursor.fetchone() apparently returns tuple and I'm assuming you do not want to pass a tuple as notify_email parameter so you are probably missing a [0] after the fetchone() when reading the admin_email.

Caveat: I am not a python programmer. And haven't tried the program below.
Also the refactored code still needs some Inversion of control refactoring, but ignore this if you do not already know what that means from other languages.

```
def get_option(cnx, blog_id, option_name):
table_name = 'wp_' + str(blog_id) + '_options'
query_ = "SELECT option_value FROM " + table_name + " WHERE option_name=%s"
with closing( cnx.cursor() ) as cursor_:
cursor_.execute(query_, (option_name,))
return cursor_.fetchone()[0]

def get_timespan():
date_to = datetime.datetime.now()
# if it monday lets get activity since saturday
if date_to.weekday() is 0:
date_from = date_to - datetime.timedelta(days=2)
# else just get the activity from the past day
else:
date_from = date_to - datetime.timedelta(days=1)
return (date_from, date_to)

def export_blog_activity(blog_id, cnx, mandrill_client):
notify_email = get_option(cnx, blog_id, 'admin_email')
encoded_result = get_option(cnx, blog_id, 'wysija')
decoded_result = phpserialize.unserialize(encoded_result.decode('base64', 'strict'))
sender_email = decoded_result['from_email']
date_from, date_to = get_timespan()
mandrill_client.exports.activity(notify_email, date_from, date_to, None, sender_email,'Sent')

def main():
# database name and credentials
# these should eventually be parsed from command line arguments
database_config = {
'user': 'root',
'password': 'password',
'database': 'wpmudb'
}
# Mandrill API Key
# this should also be parsed from command line
mandrill_api_key = 'xxxxx_xxxxxxxxxxxxxxxx'
mandrill_client = mandrill.Mandrill(mandrill_api_key)
with closing(mysql.connector.connect(**database_config)) as cnx:
export_activity = functools.p

StackExchange Code Review Q#44539, answer score: 6