views:

47

answers:

2

Hi,

I have developed a few Django apps, all pretty straightforward in terms of how I am interacting with the models.

I am building one now that has several different views which for lack of a better term are "canned" search result pages. These pages all return results from the same model, but they are filtered on different columns. One page we might be filtering on type, on the other we might be filtering on type and size, and on yet another we may be filtering on sizeonly, and so on and so forth.

I have written a function in views.py which is used by each of these pages, it takes a kwargs and in that are the criteria upon which to search. The minimum is one filter but one of the views has up to 4.

I am simply seeing if the kwargs dict contains one of the filter types, if so I filter the result on that value (I just wrote this code now, I apologize if any errors, but you should get the point):

def get_search_object(**kwargs):

  q = Entry.objects.all()

  if kwargs.__contains__('the_key1'):
    q = q.filter(column1=kwargs['the_key1']) 

  if kwargs.__contains__('the_key2'):
    q = q.filter(column2=kwargs['the_key2']) 

  return q.distinct()

Now, according to the django docs (http://docs.djangoproject.com/en/dev/topics/db/queries/#id3), these is fine, in that the DB will not be hit until the set is evaluated, lately though I have heard that this is not the most efficient way to do it and one should probably use Q objects instead.

I guess I am looking for an answer from other developers out there. My way currently works fine, if my way is totally wrong from a resources POV, then I will change ASAP.

Thanks in advance

+2  A: 

In this case, your usage is fine from an efficiency standpoint. You would only need to use Q objects if you needed to OR your filters instead of AND.

Chris Lawlor
As a side note, you may want to consider using the excellent django-toolbar (http://github.com/robhudson/django-debug-toolbar) among other things, it analyzes the SQL queries needed to create each page you visit. You can easily use this information to help in testing out various possible query implementation. Remember, "In theory there is no difference between theory and practice. But, in practice, there is." - coincidentally that quote is from the book "SQL Performance Tuning"
Chris Lawlor
I will check that out, thanks.
picus
+3  A: 

Resource-wise, you're fine, but there are a lot of ways it can be stylistically improved to avoid using the double-underscore methods and to make it more flexible and easier to maintain.

If the kwargs being used are the actual column names then you should be able to pretty easily simplify it since what you're kind of doing is deconstructing the kwargs and rebuilding it manually but for only specific keywords.

def get_search_object(**kwargs):
    entries = Entry.objects.filter(**kwargs)
    return entries.distinct()

The main difference there is that it doesn't enforce that the keys be actual columns and pretty badly needs some exception handling in there. If you want to restrict it to a specific set of fields, you can specify that list and then build up a dict with the valid entries.

def get_search_object(**kwargs):
    valid_fields = ['the_key1', 'the_key2']
    filter_dict = {}
    for key in kwargs:
        if key in valid_fields:
            filter_dict[key] = kwargs[key]
    entries = Entry.objects.filter(**filter_dict)
    return entries.distinct()

If you want a fancier solution that just checks that it's a valid field on that model, you can (ab)use _meta:

def get_search_object(**kwargs):
    valid_fields = [field.name for field in Entry._meta.fields]
    filter_dict = {}
    for key in kwargs:
        if key in valid_fields:
            filter_dict[key] = kwargs[key]
    entries = Entry.objects.filter(**filter_dict)
    return entries.distinct()
Daniel DiPaolo
Hmm, interesting, thanks for the responses.
picus