-2

A lot of times I find myself filtering for an object and returning None if it can't be found. However, the method I do this seems really inefficient (in terms of lines of code)

When I filter for an object I normally do something like this:

person = Person.objects.filter(id=id)
if person:
    person = Person.objects.get(id=id)
else:
    person = None

Is there a better way of doing this?

Edit I have made edits to clarify confusion on my end. The filter query should always return 1 object, if it exists.

John Smith
  • 843
  • 2
  • 9
  • 21

5 Answers5

3

Just use .get() if you want to get one person or return None.

try:
    person = Person.objects.get(name=name)
except (Person.DoesNotExist, Person.MultipleObjectsReturned) as e:
    person = None
Craig Collins
  • 101
  • 5
  • it is really bad idea, because Exception will be raised even if it will be some problem with db (connection, etc.). And the person could easily be there, but you return None. – Daniel Barton Aug 17 '16 at 18:30
  • I see, I'll fix it. – Craig Collins Aug 17 '16 at 18:44
  • Is there a one line solution for this? It sees like this is a bit of code for something that is a common query. – John Smith Aug 21 '16 at 03:18
  • Not to sure about a 1 line solution, but using `try except` follows python style guidelines over `if/else` format. https://docs.python.org/2/glossary.html#term-eafp – Craig Collins Aug 31 '16 at 18:40
3

You cannot do like this. 

person = Person.objects.get(name=name)

will raise an exception.

What you can do is:

try:
   person = Person.objects.get(name=name)
except Person.MultipleObjectsReturned:
   person = Person.objects.first()
except Person.DoesNotExist:
   person = None

But Best thing here is to use:

some_queryset.filter(pk=entry.pk).exists()
Aamish Baloch
  • 1,200
  • 12
  • 9
1

Your if/else is unusual in that you assign person twice. I can't understand why. You have two options I think.

First, you could reduce if/else to just if like this:

person = Person.objects.filter(name=name)
if not person:
    person = None

Or with a coalescing operator to make it very terse:

person = Person.objects.filter(name=name) or None

Which will return person = None if Person.objects.filter(name=name) is falsy.

Community
  • 1
  • 1
roganjosh
  • 12,594
  • 4
  • 29
  • 46
  • This would work great if I could use `.get` and the query didn't crash when None is expected to be returned. – John Smith Aug 19 '16 at 21:30
  • @JohnSmith the edit to your question has made it entirely different. Originally you repeated the `filter` in `if/else` so my 1-liner is exactly the same as what you were doing originally. Now it doesn't make sense. – roganjosh Aug 20 '16 at 10:15
1

Filter return list (or empty list), so if you know you get list, and want to replace empty list with None:

persons = Person.objects.filter(name=name)
if not any(person):
    person = None
# single person
person = persons[0] # but there could be more than one

If you want single Person

try:
   person = Person.objects.get(name=name)
except Person.MultipleObjectsReturned:
   # do something if there is more Persons with that name
   person = Person.objects.first() # for example return first person with that name
except Person.DoesNotExist:
   person = None # set person None
Daniel Barton
  • 491
  • 5
  • 14
  • Why would you need `any()` here? It's an empty list, which is falsy. `if not person`? – roganjosh Aug 17 '16 at 18:09
  • if it is not empty it will iter full list -> so it will be slower if there is more than one person, **any** function return True if there is person[0] or False if ther is no person[0] – Daniel Barton Aug 17 '16 at 18:12
  • I'm not sure I understand. `[]` will always return false, and nothing has to be iterated because the list is simply empty. A list with items in it will return `True`, but it shouldn't have to iterate the entire list to do the check, surely? – roganjosh Aug 17 '16 at 18:14
  • But it's a list. An empty list is itself, by default, considered `False` in Python. `print bool([])`. It doesn't matter about the nature of its contents; if it's empty, it's false. – roganjosh Aug 17 '16 at 18:18
  • I'm not arguing this for the sake of it btw. I don't think I've seen `any()` used in this scenario and since you're suggesting that it causes it to only check the first element of the list rather than iterate the entire list to return `True`, I have some investigation to do :) – roganjosh Aug 17 '16 at 18:23
  • the result is same, but any is little bit faster: – Daniel Barton Aug 17 '16 at 18:25
  • try create for example 999999 persons and in ipython run: `%timeit if any(Person.objects.all()): x=1` and `%timeit if Person.objects.all(): x=1` and the second one will be little bit slower. So yes maybe in this example it does not matter, but I usually use 'any' for checking collections. – Daniel Barton Aug 17 '16 at 18:28
  • Missed your comment as I was setting up my own test case, which I have run on loops lasting ~5 secs each. No distinguishable difference between them on `100000` list content checks, each containing 1000 items. Since it's calling a built-in on each iteration but causing no obvious overhead I will accept that there could be situations that this might be faster (or maybe `if not` does this under the hood anyway). Not related to question, but +1 for identifying this for me and teaching me something. – roganjosh Aug 17 '16 at 18:39
1

You can use exists()

From the docs:

If you only want to determine if at least one result exists (and don’t need the actual objects), it’s more efficient to use exists().

entry = Entry.objects.get(pk=123)
if some_queryset.filter(pk=entry.pk).exists():
    print("Entry contained in queryset")
else:
   return None

You can shorten this a little if lines of code is a concern. However:

Additionally, if a some_queryset has not yet been evaluated, but you know that it will be at some point, then using some_queryset.exists() will do more overall work (one query for the existence check plus an extra one to later retrieve the results) than simply using bool(some_queryset), which retrieves the results and then checks if any were returned.

mateuszb
  • 1,083
  • 10
  • 20
Paul Collingwood
  • 9,053
  • 3
  • 23
  • 36