2

I'm trying to build a search page that will allow a user to find any instances of a model which meet certain threshold criteria and am having trouble avoiding seriously redundant code. I'm hoping there's a better way to do it. Here's a slightly contrived example that should illustrate what I'm trying to do, with the relevant code adjusted at the end. The user will interact with the search with checkboxes.

models.py:

class Icecream(models.Model()):
    name = models.CharField()
    bad_threshold = models.IntegerField()
    okay_threshold = models.IntegerField()
    tasty_threshold = models.IntegerField()
    delicious_threshold = models.IntegerField()

views.py:

def search_icecreams(request):
    user = request.user
    q_search = None

    if 'taste_search' in request.GET: 
        q_search = taste_threshold_set(request, user, q_search)

    if q_search == None:
        icecream_list = Icecream.objects.order_by('name')
    else:
        icecream_list = College.objects.filter(q_search)

    context = { 'icecream_list' : icecream_list }
    return render(request, '/icecream/icecreamsearch.html', context)

The relevant code that I want to cut down is as follows, this is pretty much straight from my project, with the names changed.

def taste_threshold_set(request, user, q_search):
    threshold = request.GET.getlist('taste_search')
    user_type_tolerance = user.profile.get_tolerance_of(icea

    # 1-5 are the various thresholds. They are abbreviated to cut down on the
    # length of the url.
    if '1' in threshold:
        new_q = Q(bad_threshold__gt = user.profile.taste_tolerance)        

        if q_search == None:
            q_search = new_q
        else:
            q_search = (q_search) | (new_q)

    if '2' in threshold:
        new_q = Q(bad_threshold__lte=user.profile.taste_tolerance) & \
            ~Q(okay_threshold__lte=user.profile.taste_tolerance)

        if q_search == None:
            q_search = new_q
        else:
            q_search = (q_search) | (new_q)

    if '3' in threshold:
        new_q = Q(okay_threshold_3__lte=user.profile.taste_tolerance) & \
            ~Q(tasty_threshold__lte=user.profile.taste_tolerance)

        if q_search == None:
            q_search = new_q
        else:
            q_search = (q_search) | (new_q)

    if '4' in threshold:
        new_q = Q(tasty_threshold__lte=user.profile.taste_tolerance) & \
            ~Q(delicious_threshold__lte=user.profile.taste_tolerance)

        if q_search == None:
            q_search = new_q
        else:
            q_search = (q_search) | (new_q)

    if '5' in threshold:
        new_q = Q(delicious_threshold__lte = user.profile.taste_tolerance)        

        if q_search == None:
            q_search = new_q
        else:
            q_search = (q_search) | (new_q)

    return q_search

Basically I want the user to be able to find all instances of a certain object which meet a given threshold level. So, for example, all icecreams that they would find bad and all icecreams that they would find delicious.

There are a number of things I'm not happy with about this code. I don't like checking to see if the Q object hasn't been instantiated yet for each possible threshold, but don't see a way around it. Further, if this were a non-django problem I'd use a loop to check each of the given thresholds, instead of writing each one out. But again, I'm not sure how to do that.

Finally, the biggest problem is, I need to check thresholds for probably 20 different attributes of the model. As it stands, I'd have to write a new threshold checker for each one, each only slightly different than the other (the name of the attribute they're checking). I'd love to be able to write a generic checker, then pass it the specific attribute. Is there any way to solve this, or my other two problems?

Thanks!

Tomasz Jakub Rup
  • 10,502
  • 7
  • 48
  • 49
John Lucas
  • 588
  • 5
  • 20
  • is this code working ? `(q_search)` looks invalid unless the parameter `q_search` is a `Q` object – karthikr Jun 20 '13 at 21:16
  • Are you talking about the third code block? It's slightly modified from my code (which is working), so I might've broke something, but I think it works. The q_search parameter will either be None or a Q object, but it always checks to see if it is None. If it is, it names the new_q q_search and moves on, if it isn't it ors the old and the new, saving the result. Well, that's the goal, at least. – John Lucas Jun 20 '13 at 21:33

2 Answers2

1

You should use own QuerySet for the models instead def taste_threshold_set(...)

Example:

models.py:
...
from django.db.models.query import QuerySet
...

class IcecreamManager(models.Manager):

    def get_query_set(self):
        return self.model.QuerySet(self.model)

    def __getattr__(self, attr, *args):
        try:
            return getattr(self.__class__, attr, *args)
        except AttributeError:
            return getattr(self.get_query_set(), attr, *args)


class Icecream(models.Model()):
    name = models.CharField()
    bad_threshold = models.IntegerField()
    okay_threshold = models.IntegerField()
    tasty_threshold = models.IntegerField()
    delicious_threshold = models.IntegerField()

    objects = IcecreamManager()

    class QuerySet(QuerySet):
        def name_custom_method(self, arg1, argN):
            # you must rewrite for you solution
            return self.exclude(
                            time_end__gt=now()
                        ).filter(
                            Q(...) | Q(...)
                        )

        def name_custom_method2(...)
            ...

These should give you abilities build of chains querys for your issues.

Abbasov Alexander
  • 1,848
  • 1
  • 18
  • 27
  • Thanks @Abbasov, that probably is a better place to put the logic, instead of in the view. Though I'm a little confused how to then interact with the custom QuerySet. Could you elaborate a bit? – John Lucas Jun 21 '13 at 14:23
  • It simple as usual in Django `Icecream.objects.name_custom_method2(...).name_custom_method(...).filter(...)` or `Icecream.objects.filter(...).name_custom_method2(...).name_custom_method(...)` In other words, you will can does an any chains from Django's and own QuerySet's functions for search the result – Abbasov Alexander Jun 21 '13 at 14:41
  • Wouldn't I have to define a custom manager to do it this way? As it stands I get the error `AttributeError: 'Manager' object has no attribute 'custom_method'` – John Lucas Jun 21 '13 at 15:48
  • I am sorry, I forgot initial `IcecreamManager`. I updated code above – Abbasov Alexander Jun 21 '13 at 15:56
  • Thanks for the advice. It, combined with Wonil's answer, provides a very elegant solution to all three of my problems. I'd throw you a +1 if I could. – John Lucas Jun 21 '13 at 16:28
1

How about this approach?

query_arg = ['bad_threshold__lte', 'bad_threshold__lte', 'okay_threshold_3__lte', 'tasty_threshold__lte', 'delicious_threshold__lte']

Q(**{query_arg[int(threshold) - 1]: user.profile.taste_tolerance})
Wonil
  • 6,364
  • 2
  • 37
  • 55
  • Very nice! This is pretty close to exactly what I was looking for. I'm guessing I should be able to pass in the `user.profile.taste_tolerance` section as well, which should allow for it to be generalized amongst all attributes. So I guess this means `attribute_name__lte` is a keyword parameter for all Q objects and query sets? That's very interesting. Thanks! – John Lucas Jun 21 '13 at 13:45