twoscoops / two-scoops-of-django-1.8

Tracking thoughts and feature requests for Two Scoops of Django 1.8 in the issue tracker. And the book's code examples are here.
400 stars 81 forks source link

Improvement of chapter_07_example_3 #140

Closed lampslave closed 7 years ago

lampslave commented 8 years ago

I think this code is much clearer than current version:

# Don't do this!
from django.models import Q

from promos.models import Promo

def fun_function(**kwargs):
    """Find working ice cream promo"""
    results = Promo.objects \
        .active() \
        .filter(Q(name__startswith=name) |
                Q(description__icontains=name)) \
        .exclude(status='melted') \
        .select_related('flavors')
    return results
pydanny commented 7 years ago

Hmmm... I'm on the fence. Our habit is to break it up into multiple variable assignments, but I see that your convention is valid too. Perhaps we should mention both approaches?

lampslave commented 7 years ago

Perhaps we should mention both approaches?

Would be great :)

I use this convention because I prefer long names (this code is synthetic but not impossible):

current_user_agency_tasks = AgencyTask.objects \
    .filter(agency__employee_profiles__user=current_user) \
    .exclude(status__in=[AgencyTask.STATUS_DONE, 
                         AgencyTask.STATUS_EXPIRED]) \
    .prefetch_related('project__employee_profiles__groups')

vs

currenct_user_agency_tasks = AgencyTask.objects.filter(
    agency__employee_profiles__user=current_user)
currenct_user_agency_tasks = currenct_user_agency_tasks.exclude(
    status__in=[AgencyTask.STATUS_DONE, AgencyTask.STATUS_EXPIRED])
currenct_user_agency_tasks = currenct_user_agency_tasks.prefetch_related(
    'project__employee_profiles__groups')

vs

current_user_agency_tasks = AgencyTask.objects.filter(
    agency__employee_profiles__user=current_user
).exclude(
    status__in=[AgencyTask.STATUS_DONE, 
                AgencyTask.STATUS_EXPIRED]
).prefetch_related(
    'project__employee_profiles__groups'
)

vs

currenct_user_agency_tasks = AgencyTask.objects.filter(agency__employee_profiles__user=current_user)
currenct_user_agency_tasks = currenct_user_agency_tasks.exclude(status__in=[AgencyTask.STATUS_DONE, AgencyTask.STATUS_EXPIRED])
currenct_user_agency_tasks = currenct_user_agency_tasks.prefetch_related('project__employee_profiles__groups')
pydanny commented 7 years ago

Nicely stated! Could you add your real name to your GitHub profile so we can provide proper accreditation for your suggestion?

lampslave commented 7 years ago

Yes, sure. Thanks a lot.

pydanny commented 7 years ago

Finally added as a legibility alternative! Yeah!

🚢 :shipit: 🍨