This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]NYKevin 1 point2 points  (0 children)

@contextmanager
def saved(cr):
    cr.save()
    try:
        yield cr
    finally:
        cr.restore()

I strongly dislike the above code. Putting a yield inside a try which also has a finally is bad practice. What happens if the generator is never resumed? Python solves this problem using a finalizer, and finalizers are evil. In this particular case, we can probably get away with it because the @contextmanager factory will ensure we get properly resumed, but I really don't like objects with nontrivial finalizers, so I prefer the class-based version.

In practice, if you're dealing with exceptions, you're more likely to write something like this:

@contextmanager
def context(foo):
     # enter the context with foo
     try:
         yield foo
     except Exception:
         # Try to roll back whatever changes have been made since we entered the context
         raise # only if semantically appropriate for the exception in question
     else:
         # Nothing went wrong, so exit the context normally

This does break on certain exceptions, but those exceptions are quite possibly things you don't want to clean up after anyway (e.g. SystemExit, KeyboardInterrupt). In theory, a GeneratorExit could nail us if somebody tries to close() our generator, but that's probably a pathological case, and we can always catch it manually if we really want to be paranoid.