7
\$\begingroup\$

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()
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

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

  1. 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.
  2. 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.
  3. 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.
  4. 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.
  5. 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.
  6. Just a thought: since a getter method is used for __pipes, perhaps you can consider a setter method as well.
\$\endgroup\$
3
  • \$\begingroup\$ 1 Changing add_pipe to __add_pipe will improve safety but will reduce the functionality. \$\endgroup\$
    – Gurman
    Commented Nov 27, 2014 at 23:18
  • \$\begingroup\$ 4 and 5 It is splitter not joiner. Just don't initialise OutSplitter by stderr, etc. Or do the redirection outside a program. \$\endgroup\$
    – Gurman
    Commented Nov 27, 2014 at 23:24
  • \$\begingroup\$ 6 This is a controversial issue \$\endgroup\$
    – Gurman
    Commented Nov 27, 2014 at 23:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.