5

It seems object cleanup is a pretty common problem I've encountered during my programing days. To date I have consistently used the with statement as recommended here

I had another thought today which seems more elegant to me (because it does not require a with statement by the end user). The idea would be to use a try-finally decorator for objects of a certain type (have a cleanup method).

Just wondering if there is anything wrong with this practice or if there is something even better. I don't like that many of my classes need to be initialized using a with statement, but I also want to ensure my objects are closed properly. Here's a short example.

def cleanme(func):
    def _decorator(self, *args, **kwargs):
        try:
            func(self, *args, **kwargs)
        finally:
            self._cleanup()

    return _decorator


class IObject(object):
    def __init__(self):
        self.file_name = "some_file.txt"
        self._file_object = None
        self._cleaned = True

    @cleanme
    def run(self):
        self._connect()
        while True:
            # do some things over a long period
            pass

    def _connect(self):
        self._file_object = open(self.file_name)
        self._cleaned = False

    def _cleanup(self):
        if not self._cleaned:
            self._file_object.close()
            self._cleaned = True
Community
  • 1
  • 1
Paul Seeb
  • 6,006
  • 3
  • 26
  • 38
  • 1
    The point of 'with' is to get away from this "cleanup everything later" idea and explicitly state which resources you use. You should split your object in two parts, one that is a resource (hold that file objects and knows how to clean up) and another part that uses it, the 'run' method should be "with my_resources: while True: do_stuff()". – Jochen Ritzel Jul 22 '14 at 15:19
  • That gets extremely messy when using many IO style objects (say I have 3 sockets open to recieve data, a video feed object and some communication pipes, etc.). Instead I can just have a connect method and a close method that handles each one. – Paul Seeb Jul 22 '14 at 15:24
  • 2
    I guess this handles that problem: http://stackoverflow.com/questions/3024925/python-create-a-with-block-on-several-context-managers – Paul Seeb Jul 22 '14 at 15:56

2 Answers2

3

Let me shoot a couple of holes in this.

One thought, you're obliging your classes to have a cleanup(), which is declared not in a place different from run(). So you're imposing a user's responsibility to implement and maintain it cleanly.

Destructors other than __exit__ are pretty uncommon in Python, so the resource acquisition code may drift from cleanup() code, imposing a leak.

Second, you're making file_object an instance variable, thus widening its scope from a single function, which is also somewhat bad.

Victor Sergienko
  • 13,115
  • 3
  • 57
  • 91
2

I think this approach is ok as long as your client will only use your IOObject to do whatever run does, and won't want to call connect directly and then do some other operations using the file_object that got opened.

Context managers make it easy for the client to know exactly where they're acquiring/cleaning up a resource, and also gives them the flexibility to essentially do whatever they want with it, knowing that as soon as they leave the with block it will be cleaned up. With this approach it's less clear; a client would need to look at the code (or perhaps the documentation) to know that the run method will clean up for them, but that if connect is used directly, cleanup will need to be called to properly clean up.

There's also the Zen of Python rule "Excplicit is better than Implicit". The with statement explicitly shows the client where a resource is being acquired and release. You lose that with the decorator approach.

dano
  • 91,354
  • 19
  • 222
  • 219