HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Output splitter to handle output

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handleoutputsplitter

Problem

This class is helping to manipulate by output streams stderr, stdout, and file. Are there any cons and errors in this code?

class OutSplitter(object):
""" splitter """

def __init__(self, out_splitter):
    self.__pipes=None
    if isinstance(out_splitter, OutSplitter):
            self.__pipes = Set(out_splitter.pipes)
    elif out_splitter:
        if hasattr(out_splitter,"__iter__") : #
            for pn in out_splitter:
                self.add_pipe(pn)
        else:
            self.add_pipe(out_splitter)

@property
def pipes(self):
    return self.__pipes

def add_pipe(self, pn):
    if isinstance(pn, basestring):
        if pn == 'stderr': x = sys.stderr
        elif pn == 'stdout': x = sys.stdout
        else: 
            p = os.path.dirname(pn)
            if p and not os.path.exists(p):
                os.makedirs(p)
            x = open(pn,'w+')
    else: x = pn

    if hasattr(x, 'write') and hasattr(x, 'flush'):
        if self.__pipes is None:
            self.__pipes=Set()
        self.__pipes.add(x)

def write(self,s):
    if self.__pipes:
        for p in self.__pipes:
            p.write(s)

def flush(self):
    if self.__pipes:
        for p in self.__pipes:
            p.flush()

Solution

Overall, I find the code pretty good. A couple of suggestions from my side:

  • self.__pipes=Set(): I think add_pipe() should not be doing this. When the object is created it should be initialized fully. So this line should be placed inside __init__() at the start of the elif out_splitter: part.



  • x = open(pn,'w+'): What if this call fails? Exceptions should be handled. There is a good chance that someone using this class may trigger file open failure because they may instantiate the same object from another object (constructor is allowing this). If you make low-level stuff easy to do, you should also provide for exception handling where things might go wrong.



  • Files opened are not closed. In fact, it might be a good idea to open/close file handles using __enter__() and __exit__() methods, which need to be defined for this class.



  • One of the things I find with file I/O is that users want control on the order in which messages are printed. The application may throw out stdout and stderr prints in a certain order and the user might want to preserve that order. Unfortunately, the current code flushes the queue of one pipe first before moving on to the next pipe. Users of this class might find this to be inadequate.



  • In addition, users might want stdout and stderr to be redirected to files. This is not possible with the current class. It maybe okay now, but sooner or later someone will want this and they will start looking for alternatives.



  • Just a thought: since a getter method is used for __pipes, perhaps you can consider a setter method as well.

Context

StackExchange Code Review Q#70961, answer score: 6

Revisions (0)

No revisions yet.