# Class for comparing objects in Python

Im curious how to improve this code. I don't like creating objects of four different types of reporters, because this could provide some problems with testing. How about the rest of this code, for example the if-statement below?

EDIT: This is a fragment of application to measure loading time of our website and few other competitors. If our site is slower than one of competitors application should send email, if our site is two times slower then application should send message via sms.

"""Module containing classes for comparing objects"""
import senders
import reporters

class Comparator:
"""Class for comparing objects"""

def compare(self, compared, competitors):
reporter = reporters.Reporter()
rules = ComparatorExtensions(compared, competitors, reporter)

send_with_email = senders.SendWithEmail()
send_with_sms = senders.SendWithSms()
save_to_file = senders.SendToFile()
show_on_console = senders.ShowOnConsole()

if rules.rule_valid_compared():
if rules.rule_compared_smaller():
send_with_email.send(reporter.format_report())

if rules.rule_compared_two_times_smaller():
send_with_sms.send(reporter.format_report())

report = reporter.format_report()
save_to_file.send(report)
show_on_console.send(report)

class ComparatorExtensions:
"""Class for extensions for comparator class"""

def __init__(self, compared, competitors, reporter):
self.compared = compared
self.competitors = (item for item in competitors if self.valid_item(item))
self._reporter = reporter

def valid_item(self, item):
if not result:
self._reporter.report_invalid(item)
return result

def rule_compared_smaller(self):
result = False
for item in self.competitors:
self._reporter.report_smaller(self.compared, item)
result = True

return result

def rule_compared_two_times_smaller(self):
result = False
for item in self.competitors:
self._reporter.report_smaller_two_times(self.compared, item)
result = True

return result

def rule_valid_compared(self):
rule_valid = self.valid_item(self.compared)
if rule_valid:
return rule_valid


• Welcome to Code Review! Can you provide example usage or a description of what it's supposed to do? You've included your repository, but questions are supposed to be as stand-alone as possible. Besides, the current README doesn't explain much either. Thanks! – Mast Jul 3 '19 at 12:36
• This is a fragment of application to measure loading time of our website and few other competitors. If our site is slower than one of competitors application should send email, if our site is two times slower then application should send message via sms. – Adam Borowski Jul 3 '19 at 13:06

## Use functions whenever possible

I think there are too many classes and you should consider using functions instead: it is easier to test (unit testing).

## Use a logger

First thing: the code below make me think that you should use a logger instead of reinventing the wheel:

report = reporter.format_report()
save_to_file.send(report)
show_on_console.send(report)


Here is how you can define a root logger which can log in the console as well as in a file:

import logging

def setup_logger(log_dir, log_file_name):
# root logger
logger = logging.getLogger()

formatter = logging.Formatter(

file_hdlr = logging.FileHandler(
"{0}/{1}".format(log_dir, log_file_name))
file_hdlr.setFormatter(formatter)

console_hdlr = logging.StreamHandler()
console_hdlr.setFormatter(formatter)

logger.setLevel(logging.DEBUG)


This setup_logger function should be called at program startup, in your main, for instance:

if __name__ == "__main__":
setup_logger("my/work/dir", "my_app.log")


You can use this logger anywhere: in each Python module, you can define a global variable named LOG, and use it like this:

LOG = logging.getLogger(__name__)

def my_function():
LOG.info("log message")


See the How To documentation

## wrong usage of generator

In your, competitors is a generator, not a tuple. After having iterate a generator the first time, then the generator is exhausted. It means that the second loop will end immediately.

self.competitors = (item for item in competitors if self.valid_item(item))


The call to rule_compared_smaller will iterate the competitors, then the call to rule_compared_two_times_smaller will do nothing because the competitors is exhausted.

You need to create a list:

self.competitors = [item for item in competitors if self.valid_item(item)]


According to pep8 imports should be ordered alphabetically so

import reporters
import senders


Your classes from the senders module don't take instantiation parameters and just have one method used so these could probably just be functions.

from senders import send_email, send_sms, save_file, show_console


Similarily Comparator only contains a single method and no state so it too should be a function

def compare(compared, competitors):
...


valid_item and rule_valid_compared seem to have their responsibilities a bit mixed, I would change this to.

def is_item_valid(self, item):

def report_valid_item(self, item):
is_valid = self.is_item_valid(item):
if is_valid:
else:
self._reporter.report_invalid(item)
return is_valid


You can also reduce a lot of duplication between the two actual comparison methods by parametrize it, also as each item has already been validated you can replace the get with key lookup.

def compare_is_smaller(self, n=1, report_func=lambda a: None):
for item in self.competitors: