OOP noob question: Mixin properties

[Steven D'Aprano]

On Thu, 13 Dec 2012 16:20:51 -0800, Micky Hulse wrote:

> Learning things here...
> 
> In my mixin class (a version I wrote for Django) I had this (indentation
> removed for better list readability):

Indentation is important. Please don't remove it. I've added it back in 
below:

> class JSONResponseMixin(object):
>     def __init__(self):
>         self._cache = False
>         self._cache_timeout = 86400
>         self._cache_key = None
>     @property
>     def cache(self):
>         return self._cache
>     @cache.setter
>     def cache(self, value):
>         self._cache = value
>     @cache.deleter
>     def cache(self):
>         del self._cache
[...]

The use of @property here doesn't give you any benefit (except, maybe, if 
you are paid by the line of code). Better to just expose the "cache" 
attribute directly, and likewise for the others. If, in the future, you 
need to turn them into computed properties, you can easily do so. But for 
now:

class JSONResponseMixin(object):
    def __init__(self):
        self.cache = False
        self.cache_timeout = 86400
        self.cache_key = None

and you're done.


[...]
> ...which works! But, then I looked here:
> 
> <https://github.com/django/django/blob/master/django/views/generic/
edit.py#L198>
> 
> Simply doing this:

> class JSONResponseMixin(object):
>     cache = False
>     cache_timeout = 86400 # 24 hours.
>     cache_key = None

> ...works just as good! Not to mention, it's less verbose.

Yes. But keep in mind that there is a slight difference between what you 
have here and what I suggested above.

In my code above, the cache attributes are defined on a per-instance 
basis. Each instance will have its own set of three cache* attributes. In 
the version directly above, there is a single set of cache* attributes 
shared by all JSONResponseMixin instances.

Will this make any practical difference? Probably not. Most importantly, 
if you write:

instance = Api()  # class includes JSONResponseMixin
instance.cache = True

then the assignment to an attribute will create a non-shared per-instance 
attribute which overrides the class-level shared attribute. This is 
almost certainly the behaviour that you want. So this is a good example 
of using class attributes to implement shared default state.

There is one common place where the use of shared class attributes can 
lead to surprising results: if the class attribute is a mutable object 
such as a list or a dict, it may not behave the way you expect. That is 
because only *assignment* creates a new per-instance attribute:

instance.attribute = "spam spam spam"  # overrides the class attribute


while modifying the attribute in place does not:

instance.shared_list.append("spam spam spam") 
    # every instance of the class will now see the change


Apart from that Gotcha, the use of shared class attributes to implement 
default behaviour is perfectly acceptable.


> I guess I wanted to run my ideas by the Python gurus just to make sure I
> was not doing crazy stuff here...

Seems fine to me.



> The above seems better than taking the more functional approach:
> 
> return self.render_to_response(data, cache, cache_timeout, cache_key)
> 
> Then again, maybe the more functional approach is better for its
> simplicity?

I actually prefer a functional response, up to the point where I'm 
passing so many arguments to functions that I can't keep track of what's 
what. Then I refactor into a class.


> Hopefully I'm not boring ya'll to death. :D

No worries :-)



-- 
Steven