Owerwriting of class results in old data in python

Question

Recently run into following problem using python 2.7: I have class like this:

class Comment():
    def __init__(self, preComments = [], postComment = ''):
        self.__preComments = preComments
        self.__postComment = postComment

I use code like this multiple times:

# self.commentQueue holds comment.Comment()
def getQueuedComment(self):
    queuedComment = self.commentQueue
    self.commentQueue = comment.Comment()
    return queuedComment

Idea of this code is to return instance of Comment and create new instance in place, so queuing may continue.

And result is that weirdly, but each time second code is called self.commentQueue holds data from all other instances of this class(data are appended not assigned and problem occurs only with list), but as I understand that self.commentQueue = comment.Comment() should create new, empty class, by empty I mean that self.__preComments = [] and self.__postComment = ''.

Fix is to call self.commentQueue = comment.Comment([], '') instead of self.commentQueue = comment.Comment(), but I just don't understand why this happens.

That is a Python gotcha, see [“Least Astonishment” in Python: The Mutable Default Argument](http://stackoverflow.com/questions/2639915/why-the-mutable-default-argument-fix-syntax-is-so-ugly-asks-python-newbie) — BioGeek, Mar 09 '12 at 13:45

score 1 · Answer 1 · edited May 23 '17 at 11:48

1

Problem is you use mutable variable type as constructor parameter (preComments = []). See this: "Least Astonishment" and the Mutable Default Argument

edited May 23 '17 at 11:48

Community

1
1

answered Mar 09 '12 at 13:44

yedpodtrzitko

9,035
2
40
42

score 1 · Accepted Answer · answered Mar 09 '12 at 13:45

1

Never use [] as a default argument value. This expression is only evaluated once, so all calls to your function will always use the same list instance. Make it like this instead:

class Comment():
    def __init__(self, preComments = None, postComment = None):
        if preComments is not None:
            self.__preComments = preComments
        else:
            self.__preComments = []
        if postComments is not None:
            self.__postComment = postComment
        else:
            self.__postComment = []

answered Mar 09 '12 at 13:45

Constantinius

34,183
8
77
85

1

`self.__preComments = preComments or []` or something more explicit `self.__preComments = [] if preComments is None else preComments` – rplnt Mar 09 '12 at 14:05
Using an `or` isn't foolproof, as you cannot pass in a Falsy value. – Casey Kuball Mar 09 '12 at 14:12
@Darthfett Yes, that's why I wrote the second example as well. But if we expect it to be a `list` then it's ok to use or. Luckily, list in python with whatever in it is considered "True". – rplnt Mar 09 '12 at 14:54
@rpInt Explicit is better than implicit. Just thought I would leave a comment for those who might not know. (Note the fact that I didn't address you in my comment. :) ) – Casey Kuball Mar 13 '12 at 14:36

Owerwriting of class results in old data in python

2 Answers2