CODEWITHSUNDEEP
python
599644
599644

Reputation: 561

Make conditional "with" pretier

Any advice on what is the proper 'pythonic' way for the following function. Do I have to split it into two functions?

def readSomething(fp=None):
    if fp:
        return fp.read(100)
    else:
        with open('default.txt', 'r') as fp:
            return fp.read(100)

I need something like this because the readSomething function may be called from another function that may or may not have the same file open.

For example, it may be called like this at some places:

def doSomethingWithSameFile():
    with open('default.txt') as fp:
         preample = fp.read(10)
         more_data = readSomething(fb)
         ...

or like that at other places:

def init():
    data = readSomething()
    ...

Upvotes: 3

Views: 140

Answers (5)

Harvey
Harvey

Reputation: 5821

I don't think this is the right solution, but I think it's what you want. 

import contextlib

def readSomething(fp=None):
    with contextlib.ExitStack() as stack:
        if not fp:
            fp = stack.enter_context(open('default.txt'))
        return fp.read(100)

I get the impression that you're going to duplicate this logic many functions like readSomething() so I'd recommend putting the ExitStack code into a decorator and wrapping the functions where you need this behavior.

You could also use a decorator. I don't use this kind of code, so the syntax below is almost certainly incomplete, but the general idea stands:

import functools

def fallback_to_default(fn):
    @functools.wraps(fn)
    def new_fn(fp=None, *args, **kwargs):
        with contextlib.ExitStack() as stack:
            if not fp:
                fp = stack.enter_context(open('default.txt'))
            return fn(fp, *args, **kwargs)
    return new_fn

@fallback_to_default
def readSomething(fp=None):
    return fp.read(100)

Upvotes: 4

wim
wim

Reputation: 363063

To summarise the issue in plain language:

  • You may be passed an open file handle, then you want to leave it open, because it is the caller's responsibility to close that resource.
  • You might need to open your own resource, then it is your responsibility to close it.

This is the problem with accepting inhomogeneous argument types in Python. You're allowed to do that, but it can make your code a bit more ugly sometimes.

Context managers are just syntactic sugar for try/finally: 

def readSomething(fp=None):
    close_fp = False
    if fp is None:
        fp = open('default.txt')
        close_fp = True
    try:
        return fp.read(100)
    finally:
        if close_fp:
            fp.close()

To make it any more "pretty" than that, consider to change the interfaces so that you don't have to handle both reading data and doing the resource management from within the same function - refactor to make your functions have a single responsibility.

Upvotes: 3

John Y
John Y

Reputation: 14539

Honestly, the most Pythonic version of your code is probably just what you already have, except slightly cleaned up:

def readSomething(fp=None):
    if fp:
        return fp.read(100)
    with open('default.txt') as fp:
        return fp.read(100)

That preserves your original intent and functionality. It's clear and easy to read. Sure, it has a little repetition. If your example was simplified to the point that the repeated portion is too grotesque for you, then lift it out into its own function:

def complicatedStuff(buf, sz):
    # Obviously more code will go here.
    return buf.read(sz)

def readSomething(fp=None):
    if fp:
        return complicatedStuff(fp, 100)
    with open('default.txt') as fp:
        return complicatedStuff(fp, 100)

It's not Pythonic to jump through a lot of hoops just to avoid repeating yourself a little.

Upvotes: 2

L3viathan
L3viathan

Reputation: 27323

You could define a custom context manager that does something only if None is passed to it, but it might be overkill:

class ContextOrNone(object):
    def __init__(self, obj, fn, *args, **kwargs):
        if obj is not None:
            self.obj = obj
            self.cleanup = False
        else:
            self.obj = fn(*args, **kwargs)
            self.cleanup = True
    def __enter__(self):
        return self.obj
    def __exit__(self, ex_type, ex_val, traceback):
        if self.cleanup:
            self.obj.__exit__(ex_type, ex_val, traceback)

Or, using contextlib.contextmanager:

from contextlib import contextmanager

@contextmanager
def ContextOrNone(obj, fn, *args, **kwargs):
    was_none = obj is None
    try:
        if was_none:
            obj = fn(*args, **kwargs)
        yield obj
    finally:
        if was_none:
            obj.__exit__()

Once you have this defined, you can define readSomething as:

def readSomething(fp=None):
    with ContextOrNone(fp, open, 'default.txt', 'r') as fp:
        return fp.read(100)

Upvotes: 3

user69453
user69453

Reputation: 1405

This doesn't use with, and closes the default or the file passed as argument, but maybe it is still an option.

def readSomething(fp=None):
    if fp is None:
        fp = open('default.txt')
    return (fp.read(100), fp.close)

Upvotes: 0

Related Questions