Velocity Reviews

Velocity Reviews (http://www.velocityreviews.com/forums/index.php)
-   Python (http://www.velocityreviews.com/forums/f43-python.html)
-   -   Re: OOP noob question: Mixin properties (http://www.velocityreviews.com/forums/t955485-re-oop-noob-question-mixin-properties.html)

Micky Hulse 12-14-2012 12:20 AM

Re: OOP noob question: Mixin properties
 
Learning things here...

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

<snip>

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

@property
def cache_timeout(self):
return self._cache_timeout

@cache_timeout.setter
def cache_timeout(self, value):
self._cache_timeout = value

@cache_timeout.deleter
def cache_timeout(self):
del self._cache_timeout

@property
def cache_key(self):
return self._cache_key

@cache_key.setter
def cache_key(self, value):
self._cache_key = value

@cache_key.deleter
def cache_key(self):
del self._cache_key

# ...

</snip>

....which works! But, then I looked here:

<https://github.com/django/django/blob/master/django/views/generic/edit.py#L198>

Simply doing this:

<snip>

class JSONResponseMixin(object):

cache = False
cache_timeout = 86400 # 24 hours.
cache_key = None

# ...

<snip>

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

So, from my view class:

<snip>

class Api(jmix.JSONResponseMixin, BaseDetailView):

# ...

self.cache = True
self.cache_timeout = 864000
self.cache_key = 'foo:bar:baz'

# ...

return self.render_to_response(data)

</snip>

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

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?

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

M

Steven D'Aprano 12-14-2012 12:53 AM

Re: OOP noob question: Mixin properties
 
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/blo...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

Micky Hulse 12-14-2012 07:01 PM

Re: OOP noob question: Mixin properties
 
Hi Steven!!! Thanks so much for the pro help, I really do appreciate it. :)

On Thu, Dec 13, 2012 at 4:53 PM, Steven D'Aprano
<steve+comp.lang.python@pearwood.info> wrote:
> Indentation is important. Please don't remove it. I've added it back in
> below:


Yikes! Sorry about that. I won't do that again in the future. :(

Thanks for adding the indentation back. :)

> 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:


Ah, great to know! Thank you for the clarification, I needed that.

> class JSONResponseMixin(object):
> def __init__(self):
> self.cache = False
> self.cache_timeout = 86400
> self.cache_key = None
>
> and you're done.


Beautiful! That's nice. Very simple and clean (I had a feeling my
@decorator version was getting a bit "wordy").

>> 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.

> 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.


Thank you for pointing this out! I had not realized that this was the
case. I'm still kinda new to OOP in Python and Python in general (I'm
learning it through Django).

> 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.


Whoa, that's cool!

> 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


Exactly the type of details I was hoping to learn. Thank you so much
for taking the time to explain things in such detail and easy to
understand way. I really appreciate it. :)

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


Awesome! That's good to know.

> 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.


Ah, well that's good to know. There's a part of me that wants to
revert back to passing arguments to self.render_to_response(...);
after all, there's only 3 of them. On the flip side, if I hadn't
explored other options, I wouldn't have learned anything new.

Thanks a billion Steven! I owe you one.

Have a great holiday.

Cheers,
Micky


All times are GMT. The time now is 05:55 AM.

Powered by vBulletin®. Copyright ©2000 - 2014, vBulletin Solutions, Inc.
SEO by vBSEO ©2010, Crawlability, Inc.