Two Python classes to update an RRD database

The first Python class below is used in one of my current projects and is a totally different approach to how I normally would take things. Before I refactored the class I was not declaring instance-properties/variables. Instead the whole update procedure took place inside the def update(self): method (see: "The 'old style' class"). All those instance-variables where defined inside the scope of def update(self):.

This leaded to a class with a few less lines but was harder to read so I decided to split things into "areas of responsibilities" and defined separate functions/methods (See "New Style Class" - first code snippet).

I really like the new code much more than before. It's better to read and everything is taken very short and has it's own area of responsibility.

My concerns however are, that this is "too much" or not neccesarry and might lead to problems with memory management in python - which should not be the case since the ARC should recognize assigning new values and release the old one, right!?

Anyway, just leave some feedback so I can decide if I totally adapt this kind of "my new style" I am working on!

The First "New Style" Class

```
from AlphasenseOPCN2Wrapper import AlphasenseOPCN2Wrapper
from RRDLongtermDatabase import RRDLongtermDatabase
from RRDRealtimeDatabase import RRDRealtimeDatabase
from ValueTransformation import PMValueTransformationExtractor
from ValueTransformation import BinValueTransformationExtractor

class DatabaseUpdater(object):

def __init__(self):
self.rrd_real_time_database = RRDRealtimeDatabase()
self.rrd_long_term_database = RRDLongtermDatabase()
self.alphasense_opcn2_wrapper = AlphasenseOPCN2Wrapper()
self._bin_values = None
self._bin_sum_value = None
self._pm_values = None
self._histogram_value = None
self._start_up()

def _start_up(self):
self.alphasense_opcn2_wrapper.start()

def update(self):
self._get_histogram_values(

1. Review

I much prefer the "old style" version of the code. That's because there's less code to read, and the logic is easier to follow — I don't have to keep jumping back and forth among little methods which are not documented and whose purpose is not clear.

I would suggest the following changes:

-
The _start_up method is only one line long and only called from one place, so it could be inlined into __init__.

-
The BinValueTransformationExtractor class has only one method, and the purpose of the method is to compute one value, so what you actually want here is a function, not a class.

-
Similarly for PMValueTransformationExtractor.

-
It's clearer to write 1e6 rather than 1000000.0. The latter is easily confused with 100000.0 and 10000000.0.

-
range starts at 0 by default, so you can write range(16).

-
The division by sampling_period is the same every time, so could be folded into the multiplier.

-
density_factor is always the same, so it could be made into a global variable and not recomputed every time.

1. Revised code

def bin_values(histogram):
    sample_flow_rate = histogram['SFR']
    sampling_period = histogram['Sampling Period']
    multiplier = 1e6 / (sample_flow_rate * sampling_period)
    return [histogram['Bin {}'.format(i)] * multiplier for i in range(16)]

_DENSITY = 2.5
_SENSOR_DENSITY = 1.65
_DENSITY_FACTOR = _DENSITY / _SENSOR_DENSITY
_PM = (10, 2.5, 1)

def pm_values(histogram):
    return [histogram['PM{}'.format(pm)] * _DENSITY_FACTOR for pm in _PM]

class DatabaseUpdater(object):
    def __init__(self):
        self.rrd_real_time_database = RRDRealtimeDatabase()
        self.rrd_long_term_database = RRDLongtermDatabase()
        self.alphasense_opcn2_wrapper = AlphasenseOPCN2Wrapper()
        self.alphasense_opcn2_wrapper.start()

    def update(self):
        histogram = self.alphasense_opcn2_wrapper.histogram()
        pm = pm_values(histogram)
        bin = bin_values(histogram)
        bin_sum = sum(bin)
        self.rrd_long_term_database.update(pm + [bin_sum])
        self.rrd_real_time_database.update(pm + [bin_sum] + bin)

    def __del__(self):
        self.alphasense_opcn2_wrapper.stop()

StackExchange Code Review Q#131300, answer score: 3