[Tutor] Fwd: Re: Pythonic review (descriptors)

[Alan Gauld]

Always reply using Reply ALL to include the tutor list members.

On 02/05/15 05:34, Sage Hack wrote:
> Hi, thanks for checking the code.
> I'm trying to figure all your comments and update my code properly.
>
>
> On 28/04/15 10:21 AM, Alan Gauld wrote:
>>> 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.
>>
>> Normally you'd store instance specific data in the instance
>> itself not in a class variable.
> I need to cache results since they all come from network requests.
> That's how I thought would work.
> For example :
> PageTitle('http://somewebsite.com').__get__()
>
> called 3 time will only fetch the same result the first time.
> With all my descriptor classes, it's possible that the same request come
> up and I can't control the input of carry the data properly.
>
> In my case let's say 2 Targets() share the same hostname, I don't want
> to fetch the IP twice and there two class instances so keeping the data
> in the instance would not work.

I guess I don't understand why you have two Targets with the same hostname?
Thats what seems strange. If you do need multiple instances pointing at the
same host and yet also need access to all responses then a class variable
is probably correct. But the initial assumption of multiple instances
sharing
a host is what I'd question.

Could you use a single instance, perhaps storing them in a dictionary keyed
by host? Or store the instances in the class variable so that when you try
to create a new instance you get the original back (more elegant but
trickier
to code)

>> also you overwrite the
>> classvariable entry for each instance every time you call
>> the get(). Is that really what you want?
> I do not understand this comment

Your class variable dictionary is keyed by the id. so every time you
set it you lose the previous entry for that id. From what you say above
that may be what you want. I had assumed you were trying to store
the history of responses, but it appears you just want the latest..
> The other thing is that you should have docstrings for
> both the classes and methods.
> Trying looking up doc to understand docstrings but failed :P Look
> complicated hehe.

No, its very easy, just create an anonymous triple quoted string:

def foo():
    '''  here is a doc string instead of a comment, it tells you what
        the function is for, and what it returns.'''
       # function code here

> 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.
> I should explore that indeed. I'll see what I can see.
>
>
>> Another point re the PageTitle class:
>> There is not much point in calculating the id each time,
>> it could simply be set in the init(). You never change
>> the url or host.
> That's true. I changed id. Good tip.
>
-- 
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