The Django correctness section mentions using NullBooleanField instead of BooleanField(null=True). The reverse should now be advocated.

Soft-deprecation in Django 2.1 (release notes, pull request)
Hard-deprecation in Django 3.1 (release notes, pull request)

http://docs.quantifiedcode.com/python-anti-patterns/performance/not_using_iteritems_to_iterate_large_dict.html should probably also mention that using items() isn't an anti-pattern in Python in which iteritems() has been removed and items() always returns an iterator.

Recently I've been using a lot of list comprehensions in Python. They're really useful. But I noticed that a lot of the time when I wanted to convert a generator or something into a list, I'd quickly write something like [x for x in whatever] and nothing else, when just list(whatever) would suffice. I suppose it's a readability thing?

So in short:

Anti-pattern

xs = (1, 2, 3, 4)
ys = [x for x in xs]

Best practice

xs = (1, 2, 3, 4)
ys = list(xs)

Using environment variables is not a best practice. Probably reword the section to "Alternatives" an point out the downside of usinging environment variables.

This antipattern triggers in places where .extend() would yield a different result, for example mylist = [1, 2, 3] + mylist.

https://www.quantifiedcode.com/ <-- this link points to an unserious page, or you might have an issue with your page!
greetings, Alex

https://docs.quantifiedcode.com/python-anti-patterns/django/all/correctness/not_using_null_boolean_field.html says to use NullBooleanField rather than BooleanField(null=True). However, NullBooleanField is deprecated in Django 3.1.

The "Maintainablity" chapter warns against "Using single letter to name your variables", yet the "Not using dict comprehensions" example is d = {n: n * 2 for n in l}.

This page: http://docs.quantifiedcode.com/python-anti-patterns/readability/comparison_to_none.html

Is a little tautological, i.e. "it is preferred". I think it could elaborate more on why the == operator it is not preferred.

This could be used as a reference:

https://stackoverflow.com/questions/3257919/what-is-the-difference-between-is-none-and-none

Only the title exists, but nothing is written in the body of the article.

Not sure if this can be integrated, but it would be nice to see it as it's one of the most common things I encounter across a range of technologies (not just python or django)

So I recently saw this in some django code (I've made it generic)

class ItemViewSet(viewsets.ModelViewSet):
    queryset = models.Item.objects.all()
    serializer_class = ItemSerializer

    def perform_create(self, serializer):
        instance = serializer.save()
        instance.url = reverse('item-detail', args=[instance.pk], request=self.request)
        instance.save()

This causes a double-save to a single instance, which IMO is never acceptable and a sign of a bad design.

Instead I recommend adding this to ItemSerializer (basically generating every time before sending to view)

class ItemSerializer(HyperlinkedModelSerializer):
    url = HyperlinkedIdentityField(view_name='item-detail', read_only=True)

Regarding the anti-pattern of not using with to open files, if you are reading the entire contents of the file in one shot as in the examples, you can use read_text from pathlib which opens and closes the file for you internally. So it isn't always an anti-pattern to not use a context manager for reading text/bytes, in fact I would argue that

with open("file.txt", "r") as f:
    content = f.read()

is an anti-pattern for

content  = Path("file.txt").read_text()

I couldn't get python (neither 2.7, nor 3.5 or 3.7) to raise a Method could be a function error on your example (or reasonable variations, like actually calling Rectangle.area). If this is obsolete, please update the page to describe the behaviour of current python versions. Otherwise, update the example to reproduce the error.

Also, you refer to self and cls as keywords. At least in Python 3 they aren't. There is just a (pretty strong) convention to call the first parameter of an instance method self and the first parameter of a class method cls.

The best practice suggests using forward slashes, even while on Windows. An even better practice would be to use the build in os module to join paths together. This technique is also OS independent since it knows to use the correct directory separator. Furthermore, it's a part of the Python standard library and Django even sets up a BASE_DIR variable in your settings automatically when creating a project.

In your settings.py:

# Build paths inside the project like this: os.path.join(BASE_DIR, ...)
import os
BASE_DIR = os.path.dirname(os.path.dirname(__file__))

STATICFILES_DIRS = [
    os.path.join(BASE_DIR, 'static'),
    # etc...
]

We're now close to the release of Django 3.1 which will be the eighth release since 1.8.

1.8 was an LTS release available in April 2015 and supported for 3 years. Given the lack of sections for other versions, and the age, it seems pointless to keep this section on migration now.

Avoid non-Free licences!

I see that the site is no longer being taken care of so nvm it.

In this link

shouldn't it be self.length = length instead of self._length = length in the class initialization?

Disclaimer: I'm the maintainer of the plugin

As I see that one mostly each anti-pattern there is a list of references, maybe it would be worth to add a reference to the https://pypi.org/project/flake8-builtins/ flake8 plugin for the anti-pattern https://docs.quantifiedcode.com/python-anti-patterns/correctness/assigning_to_builtin.html ? This is exactly what this plugin does, check that you are not using builtin names for your variables, function names, etc etc

btw. thanks for the excellent list of anti-patterns! 🎉

http://docs.quantifiedcode.com/python-anti-patterns/performance/not_using_iteritems_to_iterate_large_dict.html

This needs a warning that this only applies to python 2 which is end-of-lifed.

I think there is a error in this section

MEDIA_URL and STATIC_URL point to the same folder.

""" settings.py """

# Media and static root are identical
STATIC_URL = 'http://www.mysite.com/static'
MEDIA_URL = 'http://www.mysite.com/media'

First of all they are not identical here.

Then in a best practice section you write:

Ensure, STATIC_URL and MEDIA_URL point to different folders.

""" settings.py """

STATIC_ROOT = '/path/to/my/static/files'
MEDIA_ROOT = '/path/to/my/media/files'

What it has to do with URL's? Probably should be:

Media and static root are identical
STATIC_URL = 'http://www.mysite.com/static'
MEDIA_URL = 'http://www.mysite.com/static'
...
# Ensure, STATIC_URL and MEDIA_URL point to different folders.
STATIC_URL = 'http://www.mysite.com/static'
MEDIA_URL = 'http://www.mysite.com/media'

Generally this section is misleading.

I understand what writer wanted to say but its not correct.

map() returns an iterator not list. Therefore in text on few places is mentioned that map()
returns list.

Therefore use case for map over list comprehension is let's say bit different and with map
can be more memory efficient.

I'm just pointing out, so that section can be bit adjusted, but rest standing.

The link from issue #102 leads to a 404.

The list built-in is overridden
Thanks for your beautiful work :)

When a function/method has arguments with a default value assigned all other callers of the function/method should explicitly specify the optional argument name.

def foo(bar=None, zoo=None):
pass

Bad:
foo(1, "lion")

Good:
foo(bar=1, zoo="lion")

Rationale:
Additional optional arguments will likely be added to the function/method in the future and the callers won't need to be hunted down and updated to account for the new optional argument added.

Example:
def foo(flag=True, bar=None, zoo=None):
pass

Bad:
foo(1, "lion") # unexpected result since flag is now assigned 1 and bar is now assigned "lion"

Good:
foo(bar=1, zoo="lion") # No change necessary - behaves the same as before

First, the "correct" example also uses backslashes. Second, the backslashes are used incorrectly, and will not compile. There are two fixes for this:
Escape the backslashes: "\path\to\my\static\files"
Use a raw string: r"\path\to\my\static\files"

Apart from being phrased the wrong way (the headline names the pattern, not the anti-pattern), it doesn't match the content.

The anti-pattern is about using is for equality tests, not about using something besides is for identity tests.

The headline should be changed to "Using is for equality testing" or something like that.

(There should probably also be an anti-pattern "Using == for identity testing" - I see stuff like x == None or (worse) x == True far more often than a misguided is)

Can u create pdf/fb2/... version of book?

http://docs.quantifiedcode.com/python-anti-patterns/performance/using_key_in_list_to_check_if_key_is_contained_in_a_list.html

def f_a():
    my_list = list(range(500))

    bool(3 in my_list)

def f_b():
    my_set = set(range(500))

    bool(3 in my_set)

gives

In [30]: %timeit f_a()
100000 loops, best of 3: 7.7 µs per loop

In [31]: %timeit f_b()
100000 loops, best of 3: 17.5 µs per loop

It is only worth paying the cost of creating a set (O(n log n)) to get the O(log n) loop up speed if you are going to be doing it many times.

Knut (http://www.talkpythontome.com/episodes/show/18/python-anti-patterns-and-other-mistakes?utm_content=buffera4f93&utm_medium=social&utm_source=twitter.com&utm_campaign=buffer#comment-2164624620) already created an ebook of the Anti-Patterns. It contains some issues we should fix first, then compile the book again and link it to our webpage.

As both articles describe the same problem, I think they should be merged into one document.

Hello
I'm python engineer, and this seems quite good document for me.
If I'm trying to translate this to another language, is there some rule, or logic for it?

Thanks.

http://docs.quantifiedcode.com/python-anti-patterns/correctness/bad_first_argument_given_to_super.html

This should address that in python3 you can use super() with no arguments and it will do the right thing.

On this page:
http://docs.quantifiedcode.com/python-code-patterns/correctness/explicit_return_in_init.html

Setting self.area in __init__() overrides the 'area' property created with the decorator. It should be changed to something like this (renamed area variable to _area):

class Rectangle:
    def __init__(self, width, height):
        self.width = width
        self.height = height
        self._area = width * height

    @property
    def area(self): # moved the logic for returning area to a separate method
        print 'in area()'
        return self._area

In some examples, we do use %s in strings ("Test %s".format(value)). The correct format though would be "Test {0}".format(value). Can you please check the examples and fix this?

Corrected a couple of spelling errors in one example

https://mail.python.org/pipermail/python-dev/2014-March/133118.html

I disagree with the position that EAFP is better than
LBYL, or "generally recommended" by Python. (Where do you get that? From
the same sources that are so obsessed with DRY they'd rather introduce a
higher-order-function than repeat one line of code? :-)

IMO, the primary motivation for EAFP over LBYL is avaoiding TOCTOU bugs.

Sorry for bothering you about that but it seems the pdf is gone.

In the pattern Accessing a protected member from outside the class could be use properties a best practice?

For example

class Rectangle(object):
    def __init__(self, width, height):
        self._width = width
        self._height = height
    
    @property
    def width(self):
        return self._width

r = Rectangle(5, 6)
# access from property
print("Width: {:d}".format(r.width))

In this way, we ensure read-only access to member and avoid side-effects because the assignment is not allowed

This issues refers to Use common abbreviations for libraries in the Python Knowledge Base.

Issue

The common abbreviation listed in the knowledge base for Scipy is import scipy as sp.

Although unmentioned, this alias conflicts with the common convention for Sympy: import sympy as sp .

Observations

I have seen statements import scipy as sc in the past. However, I find it is more common to import a submodule from Scipy, which cannot be accessed through aliases in this manner.

In general, it is recommended to import functions from submodule namespaces.

As suggested in the documentation, recommended formats include:

1. from scipy import integrate
2. import scipy.io as spio
3. from scipy.stats import distributions

I don't particularly like these suggestions, but I believe they more accurately reflect common practices than what is currently stated in the knowledge base.

Requests

1. Can the abbreviation for Sympy be added to the knowledge base?
2. Can we use an official, recommended convention for Scipy's alias?

References

• SO: Official abbreviation for: import scipy as sp/sc
• SO: Importing SciPy does not work

The entry for using generalized unpacking for updating multiple variables at once has these examples:

The "bad" example:

x = 1
y = 2
z = 0

x = y + 2  # 4
y = x - 3  # 1
z = x + y  # 5

And the "good" example:

x = 1
y = 2
z = 0

x, y, z = y + 2, x - 3, x + y  # more concise

And also makes the following claim:

The modified code below is functionally equivalent to the original code above, but this code is more concise.

Emphasis mine. However, the "good" example has different behavior as it uses the original values for each assignment:

x, y, z = y + 2, x - 3, x + y
print(x, y, z) # 4, -2, 3

I won't debate the "more concise" claim -- because it is -- but it's definitely less readable; however, adding parens goes a long way to delimited each update:

x, y, z = (y + 2), (x - 3), (x + y)

In the example

cars = Cars.objects.all()
for car in cars:
    do_something(car.make)

The call cars = Cars.objects.all() isn't retrieving ALL data from your database. It's only when the QuerySet is evaluated that a call is made to the database. In this case that is being done via the for in statement. Also upon evaluation, the data from the QuerySet is cached, so subsequent for loops do not hit the database.

If your goal is to just get the makes of all the cars, then values_list would be a better solution.

cars = Cars.objects.all().values_list('make', flat=True)
for make in cars:
    do_something(make)

set does not take multiple arguments.

In no_exception_type_specified.rst, we are presented with this example.

class DivisorTooSmallError(StandardError):
    def __init__(self, arg):
        self.args = arg

def divide(a, b):
    result = None
    try:
        # Raise an error if b is < 1
        if b < 1:
            raise DivisorTooSmallError
        result = a / b
    except DivisorToSmall:
        # b is below 1
        print "DivisorToSmall error: set result = 1"
        result = 1

    return result

I think this is an abuse of exception handling. The purpose of raising exceptions is to notify the caller of the function that execution of the program cannot be continued.

In this case, the exception is thrown and caught in the same method, which I think in itself is an anti-pattern, because the caller is itself. Also, if b is less than 0, or is 0, we should be throwing more specific excpetions, not setting the result to 1!

10 / 0 != 1
10 / 0 == undefined, throw an exception

I think that a better example of using exceptions effectively wihtout introducing more anti-patterns would be like the following

class DivisorTooSmallError(StandardError):
    def __init__(self, arg):
        self.args = arg


def divide(a, b):
    if b < 1:
        raise DivisorTooSmallError
    return a / b


try:
    divide(10, 0)
except DivisorTooSmallError:
    print("Unable to divide these numbers!)

In this case the exception is handled by the caller (like it should be), not in the same method that it was thrown.

https://docs.quantifiedcode.com/python-anti-patterns/maintainability/using_the_global_statement.html#encapsulate-the-global-variables-into-objects

For a rectangle, the proper term is "perimeter," not "circumference"; Circumference is reserved for circles, ovals, and other non-regular 2-D shapes with no corners. Perimeter is a sum of sides, circumference is the value of the magnitude of outer limit of a shape with no sides.
This inconsistency of the wording across this page may cause confusion to non-native speakers of English when reading the example solution provided.

The whole article is missing besides the title.

1. Using print instead of loging
2. Using root logger by logging.info or logging.getLogger() instead of logging.getLogger(__name__)
3. Configuring a logger on import
   E.g.

import logging
logger = logging.getLogger(__name__)
logger.addHandler(logging.StreamHandler(sys.stdout))

if __name__ == '__main__':
    main()

This often happens after replacing print with logging
4. Using logging.getLogger(__file__) instead of logging.getLogger(__name__)
5. Sharing a logger across multiple files
E.g.

# foo.py
LOGGER = logging.getLogger(__name__)

# bar.py
from foo import LOGGER

6. Unnecessary calculations of logging arguments

# Bad
logger.debug('Message with %s', expensive_func())

# Good
if logger.isEnabledFor(logging.DEBUG):
    logger.debug('Message with %s', expensive_func())

Docs: https://docs.python.org/3/howto/logging.html#optimization
7. Not using built-in string formatting features

# Bad
logger.info('Some data: %s', repr(data))

# Good
logger.info('Some data: %r', data)

I cannot access to the site http://docs.quantifiedcode.com/python-anti-patterns

Error 404. Not found

Comparing things to True the wrong way
Per the PEP 8 Style Guide, the preferred ways to compare something to True are the patterns if cond is True: or if cond:.

Pep8 does not recommend if cond is True; it specifically says that's even worse than == True. Implicit booleans are recommended.