Persistent, dictionary-like object ensurance

Question

This is a part of a project I'm working on. The codebase is Python 2.7 and I don't have an option to switch to Python 3.

Code:

import shelve
def get_document_info():
 token_shelf = False
 try:
 token_shelf = shelve.open(os.path.join(current_dir, 'document_info.shelve'))
 token_mapping = token_shelf['token_mapping']
 if token_mapping:
 return token_mapping
 else:
 return False
 except Exception, e:
 pass
 finally:
 if token_shelf:
 token_shelf.close()

Shelve is a persistent, dictionary-like object. More info can be found at https://docs.python.org/2/library/shelve.html

I need to ensure that token_shelf is closed at the end. In the beginning of the function, what do I define token_shelf as? If I don't define something there, PyCharm will throw an error saying that the token_shelf in finally block does not exist.

What's the ideal way to write this piece of code? How can it be made more elegant and Pythonic?

PS: This code is working as expected right now.

• Grajdeanu Alex: Welcome to codereview! What is shelve? A file? Please post all the code or at least add some context about what this code is supposed to do. And it's python 2.x or 3.x? Is this your code? So many questions...
• UnderpoweredNinja: @MrGrj: Updated the post. Is there anything else I'm missing? Please let me know.
• Grajdeanu Alex: Please add the import too. More, does this code work as expected?
• Grajdeanu Alex: This is not stackoverflow. How does a shelve file look like? What is current_dir? Where is the os imported? I don't see it in the code -> I can't run the code -> the question is off-topic. If you want help from us, you first have to let us help you by understanding all your code
• Graipher: Which exceptions are you guarding against?

Answer 1

Ideally you would want to use Python's with ... as syntax, which creates a context that calls the exit method of whatever object you create. For this, the shelve.Shelf classes would need to implement the dunder methods __enter__ and __exit__ (which they don't).

At this point you basically have three options:

1. Inherit from them and implement it yourself:

class Shelf(shelve.Shelf):
 def __new__(cls, file_name):
 return shelve.open(file_name)
 def __enter__(self):
 return self
 def __exit__(self, *args):
 self.close()

2. Or just write a simple wrapper class like this:

class Shelf(object):
 def __init__(self, file_name):
 self.file_name = file_name
 def __enter__(self):
 self.obj = shelve.open(self.file_name)
 return self.obj
 def __exit__(self, *args):
 self.obj.close()

Both can be used like this:

import os
import shelve
def get_document_info(current_dir):
 shelf_path = os.path.join(current_dir, 'document_info.shelve')
 with Shelf(shelf_path) as token_shelf:
 return token_shelf.get('token_mapping')

When you use try..except you should always guard against as specific exceptions as possible (so that unexpected exceptions can still rise to the top). In this case this would probably be a KeyError. Here I used the get method instead, which will return None if the key is not found.

I also removed the return False, because you should check for falsiness in the calling code. In other words, if you are doing if token_mapping is False or if token_mapping == False in the calling code, you are doing it wrong. Just have the if not token_mapping/if token_mapping there.

3. Use Python's contextlib.closing, which already implements something like option 2:

from contextlib import closing
import os
import shelve
def get_document_info(current_dir):
 shelf_path = os.path.join(current_dir, 'document_info.shelve')
 with closing(shelve.open(shelf_path)) as token_shelf:
 return token_shelf.get('token_mapping')

• Grajdeanu Alex: Nice. That's indeed the correct way to do what OP requested! +1
• mkrieger1: Could you explain why in the Shelf class __new__ is implemented, and not __init__?
• Graipher: @mkrieger1 Because I inherit from shelve.Shelf, so it is already implemented. Also note that the open function is not a method of the original class, but a module function, so I needed some way to return the right object upon creation. It would also have been possible to set self.file_name in __init__, though and do the opening in the __enter__ method.
• Graipher: @mkrieger1 I added the version without overwriting __new__.

Answer 2

def get_document_info():
global token_mapping
try:
 return token_mapping if (token_mapping = shelve.open(os.path.join(current_dir, 'document_info.shelve'))['token_mapping']) else None
except Exception, e:
 token_shelf.close()
 pass

---

Forcibly changed your return of False as that is bad practice as you can't substitute the shelve object everywhere unless you pre-wrap it in conditionals -- always. Honestly been a decade since I wrote anything in Python. So this is what I have pieced together from quick look-ups on tutorials online. Perhaps Python has more elegantly looking Ternary Operators (Perl is nice because it has default-ly named variables).

But, even then I never forgot that Python is a Dynamically-Typed language and that every variable is just a Reference to an instance and every instance is Strongly-typed.

• Simon Forsberg: Please keep your negative thoughts about people to yourself. Remember to be nice.