I'm working on a project where I often want to turn class instances into dictionaries to write them to MongoDB. I'd like some feedback on the following:
def getDict(self):
self.dict = dict()
for key in self.__dict__.keys():
if key not in self.NOT_INCLUDE: # class variable
try:
self.dict[key] = getattr(getattr(self, key), "getDict")()
except:
self.dict[key] = getattr(self, key)
return self.dict
This will turn any class with getDict
into a dictionary, including any instance properties that themselves have a getDict
method.
Also please let me know if there's an easy way to do this with the standard library that I missed.
2 Answers 2
What are you Returning
This function is adding a new member variable, self.dict
. Presumably you want to just return a new temporary variable. Also, dict()
is actually less efficient than {}
, so you might want to start with:
member_dict = {} # or however you want to name it
Iterating over a Dict
In python, iterating over a dict is the same as iterating over the keys, so we can just save that step:
for key in self.__dict__:
But really, you don't just want the keys, you want the values too, so let's just get both:
for key, val in self.__dict__.items():
getattr
Use getattr()
only for variable attribute names. Otherwise, just reference the attribute directly. getattr(x, 'getDict')()
is more verbose and harder to parse than x.getDict()
.
Catching Exceptions
Don't just catch every exception - catch the one you expect to see. In this case, AttributeError
:
member_dict = {}
for key, val in self.__dict__.items():
if key not in self.NOT_INCLUDE:
try:
member_dict[key] = val.getDict()
except AttributeError:
member_dict[key] = val
return member_dict
Add a Free Function
def getDictIfExists(obj):
try:
return obj.getDict()
except AttributeError:
return obj
This lets you write the whole thing as a generator expression
return {key: getDictIfExists(val)
for key, val in self.__dict__.items()
if key not in self.NOT_INCLUDE}
-
\$\begingroup\$ wow I love this. I want a full dictionary, not a generator, but otherwise these comments will definitely be adopted. \$\endgroup\$sunny– sunny2015年10月08日 15:46:58 +00:00Commented Oct 8, 2015 at 15:46
-
\$\begingroup\$ @sunny The generator expression will give you a full dictionary. \$\endgroup\$Barry– Barry2015年10月08日 15:47:56 +00:00Commented Oct 8, 2015 at 15:47
-
\$\begingroup\$ @sunny Generators only return as generator objects when they're in parentheses:
()
. When wrapped in{}
or[]
they're returning collection objects like dictionaries and lists. \$\endgroup\$SuperBiasedMan– SuperBiasedMan2015年10月08日 15:58:53 +00:00Commented Oct 8, 2015 at 15:58 -
\$\begingroup\$ @Barry Looks like it's an AttributeError rather than a NameError that should be caught. \$\endgroup\$sunny– sunny2015年10月08日 16:48:12 +00:00Commented Oct 8, 2015 at 16:48
-
\$\begingroup\$ @sunny My bad, picked the wrong one! \$\endgroup\$Barry– Barry2015年10月08日 16:55:56 +00:00Commented Oct 8, 2015 at 16:55
It doesn't even seem that necessary to have self.dict
. And if you are going to keep it, dict
is not a good attribute name. values
would be better, but it's best if you can indicate what the actual use of the dictionary is.
Likewise getDict
isn't great, get_values
would be better. And NOT_INCLUDE
is less direct and clear than EXCLUDE
. You ought to add a docstring too, that at least references the values that are excluded.
def get_values(self):
"""Return dictionary of objects attributes, excluding some values.
The excluded values are found in the EXCLUDE constant."""
-
1\$\begingroup\$
get_values
is not good either. It should beas_dict
. \$\endgroup\$CodeYogi– CodeYogi2015年10月09日 04:12:35 +00:00Commented Oct 9, 2015 at 4:12