Reduce calls to len()

[Дилян Палаузов]

if len(x) is the same as if x except when len() is supposed to throw exception when x is not iterable.

len(x) can return either zero or a positive integer, hence if len(x) < 1 is the same as if len(x) == 0 <=> if not x.

[Tim Graham]

​PR from the patch.

[Дилян Палаузов]

Even more places:

diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py
--- a/django/contrib/admin/filters.py
+++ b/django/contrib/admin/filters.py
@@ -77,7 +77,7 @@ class SimpleListFilter(ListFilter):
         self.lookup_choices = list(lookup_choices)
 
     def has_output(self):
-        return len(self.lookup_choices) > 0
+        return bool(self.lookup_choices)
 
     def value(self):
         """
diff --git a/django/db/backends/oracle/creation.py b/django/db/backends/oracle/creation.py
--- a/django/db/backends/oracle/creation.py
+++ b/django/db/backends/oracle/creation.py
@@ -268,7 +268,7 @@ class DatabaseCreation(BaseDatabaseCreation):
         """
         try:
             # Statement can fail when acceptable_ora_err is not None
-            allow_quiet_fail = acceptable_ora_err is not None and len(acceptable_ora_err) > 0
+            allow_quiet_fail = bool(acceptable_ora_err)
             self._execute_statements(cursor, statements, parameters, verbosity, allow_quiet_fail=allow_quiet_fa
             return True
         except DatabaseError as err:
diff --git a/django/db/migrations/questioner.py b/django/db/migrations/questioner.py
--- a/django/db/migrations/questioner.py
+++ b/django/db/migrations/questioner.py
@@ -85,7 +85,7 @@ class InteractiveMigrationQuestioner(MigrationQuestioner):
         result = input("%s " % question)
         if not result and default is not None:
             return default
-        while len(result) < 1 or result[0].lower() not in "yn":
+        while not result or result[0].lower() not in "yn":
             result = input("Please answer yes or no: ")
         return result[0].lower() == "y"
 
diff --git a/django/db/models/base.py b/django/db/models/base.py
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -736,7 +736,7 @@ class Model(metaclass=ModelBase):
         """
         using = using or router.db_for_write(self.__class__, instance=self)
         assert not (force_insert and (force_update or update_fields))
-        assert update_fields is None or len(update_fields) > 0
+        assert update_fields is None or update_fields
         cls = origin = self.__class__
         # Skip proxies, but keep the origin as the proxy model.
         if cls._meta.proxy:

[Aymeric Augustin]

Some changes in the patch are indeed improvements, but not all of them.

Regarding the comment above, I find that return len(self.lookup_choices) > 0 is much more explicit than return bool(self.lookup_choices) as well as much faster:

>>> import timeit
>>> timeit.timeit('len([]) > 0')
0.0808453639911022
>>> timeit.timeit('bool([])')
0.1313885069976095

[Aymeric Augustin]

The performance hit is specific to extra bool calls.

if <something> is faster than if len(<something>).

[Tim Graham]

I didn't know that len() is an O(1) operation for many types.

So it looks like for that case, the fastest option would be True if self.lookup_choices else False.

>>> timeit.timeit('True if [1] else False')
0.06975528400289477
>>> timeit.timeit('bool([1])')
0.18175133999829995
>>> timeit.timeit('len([1]) > 0')
0.12322649999987334

Is that optimizing at the expense of readability?

[Дилян Палаузов]

len() faster than bool() on lists seems to be deficiency in cpython. I wrote a report ​https://bugs.python.org/issue32180.

I think for the moment len(self.lookup_choices) > 0 can be left as it is with appropriate comment (e.g. reference to this ticket), so that one does not come to the idea to investigate whether there is place for optimizations on that place.

[Aymeric Augustin]

Yes, in my opinion True if ... else False for performance is the worse option there. I agree with Дилян.

[Дилян Палаузов]

Another option would be to change the documentation for the return type of has_output: instead of boolean, it returns a value, that can be converted to boolean equivalent to the original return value. In fact, internally has_output is called only on one place.

In db/backends/oracle/creation.py the cast to allow_quiet_fail = bool (acceptable_ora_err) is superfluous, as the called _execute_statements(... allow_quiet_fail=allow_quiet_fail)}}} just calls if not allow_quiet_fail.... Hence calling self._execute_statements(cursor, statements, parameters, verbosity, allow_quiet_fail=acceptable_ora_err) and deleting the local variable allow_quiet_fail is reasonable.

[Tim Graham <timograham@…>]

In d2afa5eb:

Fixed #28860 -- Removed unnecessary len() calls.

[Дилян Палаузов]

My comments for django/db/backends/oracle/creation.py and django/db/backends/postgresql/operations.py weren't tackled.