2
\$\begingroup\$

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.

\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

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}
\$\endgroup\$
5
  • \$\begingroup\$ wow I love this. I want a full dictionary, not a generator, but otherwise these comments will definitely be adopted. \$\endgroup\$
    – sunny
    Oct 8, 2015 at 15:46
  • \$\begingroup\$ @sunny The generator expression will give you a full dictionary. \$\endgroup\$
    – Barry
    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\$ Oct 8, 2015 at 15:58
  • \$\begingroup\$ @Barry Looks like it's an AttributeError rather than a NameError that should be caught. \$\endgroup\$
    – sunny
    Oct 8, 2015 at 16:48
  • \$\begingroup\$ @sunny My bad, picked the wrong one! \$\endgroup\$
    – Barry
    Oct 8, 2015 at 16:55
1
\$\begingroup\$

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."""
\$\endgroup\$
1
  • 1
    \$\begingroup\$ get_values is not good either. It should be as_dict. \$\endgroup\$
    – CodeYogi
    Oct 9, 2015 at 4:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.