I have seen this in both python and C#:
class Spam: def __init__(self): self.eggs = list() ...more code here... def clear(): self.eggs = list()
What do I consider wrong here? The variable ‘eggs’ is what I call ‘doubly mutable’. It is a mutable attribute of a mutable type. I hate this because I don’t know a) what to expect and b) what the usage pattern for the variable is. This confusion applies to both people using your class (if they take references to the value of the attribute, either knowingly or unknowingly), as well as coders who are maintaining your class who will have to look every time to know which pattern you follow (clearing the list, or reassigning to a new list).
IMO you should have one of the following. This first example is preferred but as it involves an immutable collection, isn’t always practical as a replacement for this antipattern (which usually involves mutating a collection).
class Spam: def __init__(self): self.eggs = tuple() #Now immutable, so it must be reassigned to change ...more code here... def clear(): self.eggs = tuple()
in which case, you know it is a mutable attribute of an immutable type. So you can’t change the value of the instance, you must reassign it.
Or you can do:
class Spam: def __init__(self): self.eggs = list() ...more code here... def clear(): del self.eggs[:] #Empty the list without reassigning the attribute.
in which case, you understand it is to be considered an immutable attribute of a mutable type. So you would never re-set the attribute, you always mutate it. If you can’t use the tuple alternative, you should always use this alternative- there’s never a reason for the original example.
In C#, we could avoid this problem entirely by using the ‘readonly’ field keyword for any mutable collection fields. In python, since all attributes are mutable, we must do things by convention (which is fine by me).
I always fix or point out this pattern when I find it and I consider it an antipattern. Does it bother you as much, or at all?