[Tutor] Pythonic review (descriptors)

Alan Gauld alan.gauld at btinternet.com
Tue Apr 28 16:21:44 CEST 2015


On 28/04/15 10:55, Sage Hack wrote:
> I'm looking for somebody willing to review parts of this code
> https://github.com/SageHack/cloud-buster and let me know what is not
> Pythonic :P

 > https://github.com/SageHack/cloud-buster/tree/master/bust/descriptor

The thing that jumps out to me is your use of class variables to hold a 
dictionary of instance responses based on the instance ID. That
pattern looks like this, and you use it in every class:

class SomeClass:
     classvar = {}

     def __init__(self,id):
        self.id = id
     def __get__(self...):
        v = getSomeValue()
        sel.classvar[self.id] = v

Normally you'd store instance specific data in the instance
itself not in a class variable. also you overwrite the
classvariable entry for each instance every time you call
the get(). Is that really what you want?

The other thing is that you should have docstrings for
both the classes and methods.

Finally, and not Python specific, You have several classes
sharing the same 'ID' data - self.domain - that's usually
a bad OOP smell. Only one class should be mastering any
given type of  data, so maybe your other classes are
really methods of whichever is the master class? Particularly
since they don't have any explicit methods (also a bad OOP
smell) of their own.


-- 
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
http://www.amazon.com/author/alan_gauld
Follow my photo-blog on Flickr at:
http://www.flickr.com/photos/alangauldphotos




More information about the Tutor mailing list