Feanor
Feanor

Reputation: 3690

Django optimize for loop in method

I wrote the method to check one attribute and return False if there is an artist with selling=False, and True otherwise.

def check_selling(track, excludes):
    """
    Returns True if all track's artists are allowed for selling. False otherwise
    """
    for artist in track.artists.all():
            if not artist.selling:
                excludes.append(track.pk)
                return False
    return True

How can i minimize this?

I tried:

def check_selling(track, excludes):
    res = excludes.append(track.pk) if not [artist.selling for artist in track.artists.all()] else True
    return res or False

But [False] or [True] resulting in list comprehension [artist.selling for artist in track.artists.all()] always gives True...

Upvotes: 2

Views: 118

Answers (3)

Leandro
Leandro

Reputation: 2247

Make Queries! See Here

def check_selling(track, excludes):
    """
    Returns True if all track's artists are allowed for selling. False otherwise
    """
    if track.artists.filter(selling=False).exists(): #If "selling" is a boolean
        excludes.append(track)
        return False
    return True

Just one query

Upvotes: 2

Sylvain Leroux
Sylvain Leroux

Reputation: 52070

premature optimization is the root of all evil -- Donald Knuth

... but, it is no question of optimization here. But of doing right.

It is highly inefficient to retrieve all records just to count those having a given attribute. You could do much better at SQL level:

SELECT COUNT(*) FROM artist WHERE SELLING != false

That statement will directly return the number of artist not selling. Not only this will reduce traffic between the RDBMS and your application -- but in some cases, the RDBMS will be able to "optimize" this statement, either by using an index (if you have one on "selling") and/or its cache. Depending your BD backend, the syntax may vary. The good new is that Django has support for such query using count(). Something like that:

artist.objects.filter(selling!=false).count()

Upvotes: 2

000
000

Reputation: 27247

http://docs.python.org/2/library/functions.html#all

New in version 2.5.

def check_selling(track, excludes):
    """
    Returns True if all track's artists are allowed for selling. False otherwise
    """
    if all( [artist.selling for artist in track.artists.all()] ):
        return True
    excludes.append(track.pk)
    return False

Upvotes: 2

Related Questions