CodeSOD: Filter Your Kwargs

Mark's team needed someone to write a pretty straight-forward report for their Python application. It would take a set of keyword arguments, turn those into a filter, apply the filter, and get the results back.

This was implemented in two methods. First:

def set_filters_to_query(modelquery, kwargs2): filter_by = {} if 'source' in kwargs2 and kwargs2['source'] == u'': del kwargs2['source'] if 'assigned_user' in kwargs2: filter_by['assigned_user'] = kwargs2.pop('assigned_user') if 'country' in kwargs2: filter_by['country'] = kwargs2.pop('country') if 'lang' in kwargs2: filter_by['lang'] = kwargs2.pop('lang') if 'affiliate_data' in kwargs2: filter_by['affiliate_data'] = kwargs2.pop('affiliate_data') if 'source' in kwargs2: filter_by['source'] = kwargs2.pop('source') return kwargs2, filter_by

So, a few things. kwargs isn't a "reserved word" or anything, but by convention, it's meant to be used to support, well, keyword arguments, denoted by a leading **, allowing the method to be invoked set_filters_to_query(some_model, filter_one=value, filter_two=value).

In the method, it still behaves as a dictionary, but it provides a convenient way to call it.

In any case, this method does two things. It clears values out of kwargs2, and moves them into filter_by. It then returns the pair of them as a tuple. Well, three things- it utterly ignores the modelquery parameter.

This whole method is a code smell, but aside from the misuse of kwargs, there's nothing that screams "WTF". How does this get invoked?

def count_leads(kwargs, verified): kwargs2 = dict(kwargs) kwargs2, filter_by = set_filters_to_query('Lead', kwargs2) if verified: kwargs2['email_verified'] = True results = defaultdict(int) total = 0 view_by = kwargs2.pop('view_by', None) kwargs2.update(filter_by) leads = Lead.objects.filter(**kwargs2) ...

Once again, we have a method that does some kwargs misuse. We also can't trust that the input is a dict (for some reason- a problem we wouldn't have if we used kwargs correctly), so we convert it into a dict then pass the result to set_filters_to_query.

Then, a few lines later, we do this: kwargs2.update(filter_by)

set_filters_to_query popped a few fields out of kwargs2 and put them in filter_by. This operation puts them right back. The only actual side effect is that we may have deleted the key source if it was an empty string.

Then finally, we call the filter, passing **kwargs2, which expands the dict back out into keyword arguments as it's clear that Lead.objects.filter was not written by the developer responsible for this code, and works the way we'd expect such a method to work.

This code was eradicated, as you'd expect, and replaced with something more reasonable and straightforward.

[Advertisement] ProGet's got you covered with security and access controls on your NuGet feeds. Learn more.