# Python file reader/writer, with processing pipeline and context manager

The following code does three things:

1. The filename is determined by the RawData class
2. The reader and writer send data through a processing pipeline
3. (Hopefully) makes changes and extensions easy

Regarding (2), the processing pipeline consists of "processors", that can process and deprocess data. For example, if you run the code below and look in ortho_animals.txt, you'll see the JSON version of a dict with (keys, values) reversed.

I left comments out to keep the size of the code to a minimum. Please let me know what you think, with a particular eye on the class designs and interactions.

import contextlib
import json

class Processor(object):

@classmethod
def process(cls, data):
raise NotImplementedError

@classmethod
def deprocess(cls, data):
raise NotImplementedError

class JsonProcessor(Processor):
@classmethod
def process(cls, data):
return json.dumps(data)

@classmethod
def deprocess(cls, data):

class SwapProcessor(Processor):
@classmethod
def process(cls, data):
return cls.swap(data)

@classmethod
def deprocess(cls, data):
return cls.swap(data)

@classmethod
def swap(cls, data):
return {v: k for k, v in data.iteritems()}

class AggregateProcessor(Processor):
def __init__(self, processors):
self.processors = processors

def process(self, data):
return reduce(lambda d, kls: kls.process(d), self.processors, data)

def deprocess(self, data):
return reduce(lambda d, kls: kls.deprocess(d), self.processors[::-1], data)

def __init__(self, file_obj, processor):
self._file = file_obj
self._processor = processor

return self._processor.deprocess(data)

class RawWriter(object):
def __init__(self, file_obj, processor):
self._file = file_obj
self._processor = processor

def write(self, data):
new_data = self._processor.process(data)
self._file.write(new_data)

class RawData(object):
processors = [SwapProcessor, JsonProcessor]
aggregator = AggregateProcessor(processors)

def __init__(self, obj_type, key):
self.obj_type = obj_type
self.key = key

def _build_filename(self):
return self.obj_type + '_' + self.key + '.txt'

@contextlib.contextmanager
def writer(self):
filename = self._build_filename()
with open(filename, 'w') as f:
yield RawWriter(f, self.aggregator)

@contextlib.contextmanager
filename = self._build_filename()
with open(filename, 'r') as f:

if __name__ == '__main__':
data = {'cats': 'suck', 'dogs': 'rule'}
rd = RawData('ortho', key='animals')

with rd.writer() as writer:
writer.write(data)

assert(new_data['cats'] == data['cats'])


This code looks very nice! I admire how clean and organized your code is. It appears that you tried to think through how to arrange it, and I really appreciate that.

There are really only a few areas that I see things could be improved: the speed of execution, the robustness of the code when a user attempts to supply invalid data, and the verbiage for readability when others must see this code.

## Efficiency

I had to look it up, but I remember reading about how reducing all of your code that follows if __name__ == '__main__': into a single main() function is good practice. After searching around I remember where I read it, it was in the Python Cookbook (3rd edition) by O'Reilly Media. The authors David Beazley and Brian K. Jones wrote:

A little-known fact is that code defined in the global scope like this runs slower than code defined in a function. The speed difference has to do with the implementation of local versus global variables (operations involving locals are faster) ... The speed difference depends heavily on the processing being performed, but in our experience, speedups of 15-30% are not uncommon

## Robustness

I can see in a couple places where someone using this code may (whether accidentally or with malicious intent) break it and cause an error, or perhaps not cause an error but make it do something not designed. You may want to consider adding checks throughout your code, so that if say a value in a dict is of a type other than a string, then you know something is wrong and you shouldn't continue with the swap function. I realize adding Exception handling throughout or if/then branches will make for less clean and readable syntax, but it is an area where you may need to take this next if you plan to put this into a production environment.