Adding method to class at run-time: bad style?
Grant Edwards
"run-time", but what I'm talking about is this:
import ClientForm
def controlEqual(self,other):
return (self.type == other.type) and \
(self.name == other.name) and \
(self.value == other.value) and \
(self.disabled == other.disabled) and\
(self.readonly == self.readonly)
def controlNotEqual(self,other):
return not (self==other)
ClientForm.Control.__eq__ = controlEqual
ClientForm.Control.__ne__ = controlNotEqual
def formEqual(self,other):
if len(self.controls) != len(other.controls):
return False
for (c1,c2) in zip(self.controls,other.controls):
if c1 != c2:
return False
return True
def formNotEqual(self,other):
return not (self==other)
ClientForm.HTMLForm.__eq__ = formEqual
ClientForm.HTMLForm.__ne__ = formNotEqual
It works fine, but it seems like it might be dangerous (or just
bad form) to insert methods into existing classes like that.
Perhaps it would be safer to make sure that __eq__, __ne__,
and __cmp__ don't exist before adding my own?
--
Grant Edwards grante Yow! NEWARK has been
at REZONED!! DES MOINES has
visi.com been REZONED!!
Michele Simionato
> I realize that technically all methods are added to classes at
> "run-time", but what I'm talking about is this:
Raymond Hettinger solves this problem nicely with a class decorator:
http://code.activestate.com/recipes/576685/
Scott David Daniels
> I realize that technically all methods are added to classes at
> "run-time", but what I'm talking about is this:
>
> import ClientForm
>
> def controlEqual(self,other):
> return (self.type == other.type) and \
> (self.name == other.name) and \
> (self.value == other.value) and \
> (self.disabled == other.disabled) and\
> (self.readonly == self.readonly)
Here you are checking that self.readonly == self.readonly!
I also dislike the trailing backslashes and over-parenthesizing.
So, I'd do it like this (using the pare to avoid the backslash):
def controlEqual(self,other):
return (self.type == other.type
and self.value == other.value
and self.disabled == other.disabled
and self.readonly == self.readonly)
...
> ClientForm.Control.__eq__ = controlEqual
> ClientForm.Control.__ne__ = controlNotEqual
>
> def formEqual(self,other):
> if len(self.controls) != len(other.controls):
> return False
> for (c1,c2) in zip(self.controls,other.controls):
> if c1 != c2:
> return False
> return True
>
> def formNotEqual(self,other):
> return not (self==other)
>
> ClientForm.HTMLForm.__eq__ = formEqual
> ClientForm.HTMLForm.__ne__ = formNotEqual
> It works fine, but it seems like it might be dangerous (or just
> bad form) to insert methods into existing classes like that.
> Perhaps it would be safer to make sure that __eq__, __ne__,
> and __cmp__ don't exist before adding my own?
Well, a more standard approach would be:
class FancyControl(ClientForm.Control):
def __eq__(self, other):
return (self.type == other.type
and self.name == other.name
and self.value == other.value
and self.disabled == other.disabled
and self.readonly == self.readonly)
def __ne__(self, other):
return not self.__eq__(other)
class FancyHTMLForm(ClientForm.HTMLForm):
def __eq__(self,other):
...
def __ne__(self, other):
return not self.__eq__(other)
You'd have to make sure FancyControl and FancyClientForm get used in
the app. The latter could be insured by monkey-patching:
...
ClientForm.Control = FancyControl
ClientForm.HTMLForm = FancyHTMLForm
But substituting monkey-patching for class method insertion seems like
you are going from one quick and dirty technique to a slightly nicer
quick and dirty technique.
--Scott David Daniels
Scott....@Acm.Org
Grant Edwards
> Grant Edwards wrote:
>> I realize that technically all methods are added to classes at
>> "run-time", but what I'm talking about is this:
>>
>> import ClientForm
>>
>> def controlEqual(self,other):
>> return (self.type == other.type) and \
>> (self.name == other.name) and \
>> (self.value == other.value) and \
>> (self.disabled == other.disabled) and\
>> (self.readonly == self.readonly)
> Note:
> Here you are checking that self.readonly == self.readonly!
Doh!
> I also dislike the trailing backslashes and over-parenthesizing.
> So, I'd do it like this (using the pare to avoid the backslash):
The paren to avoid the backslashes is a good tip.
> def controlEqual(self,other):
> return (self.type == other.type
> and self.name == other.name
> and self.value == other.value
> and self.disabled == other.disabled
> and self.readonly == self.readonly)
I don't really like to rely on my memory of operator
precidence, but that's a personal problem.
The problem is that my app never instanciates either the
Control or the HTMLForm class: that's all done by module
functions that the app calls, and they don't know about the new
classes without some trickery (e.g. below)
> The latter could be insured by monkey-patching:
> ...
> ClientForm.Control = FancyControl
> ClientForm.HTMLForm = FancyHTMLForm
>
> But substituting monkey-patching
:) How did I not know that phrase until today?
> for class method insertion seems like you are going from one
> quick and dirty technique to a slightly nicer quick and dirty
> technique.
I think like your monkey-patching solution better. It feels a
bit cleaner.
--
Grant Edwards grante Yow! ! Up ahead! It's a
at DONUT HUT!!
visi.com
Grant Edwards
> On 2009-04-07, Scott David Daniels <Scott....@Acm.Org> wrote:
>> Grant Edwards wrote:
>>> I realize that technically all methods are added to classes at
>>> "run-time", but what I'm talking about is this:
>>
>>> ClientForm.Control.__eq__ = controlEqual
>>> ClientForm.Control.__ne__ = controlNotEqual
>
>> Well, a more standard approach would be:
>> class FancyControl(ClientForm.Control):
>> def __eq__(self, other):
>> return (self.type == other.type
>> and self.name == other.name
>> and self.value == other.value
>> and self.disabled == other.disabled
>> and self.readonly == self.readonly)
>>
>> def __ne__(self, other):
>> return not self.__eq__(other)
I like that, except it doesn't work. Objects instantiated from
sub-classes of ClientForm.Control break. I get errors like
this:
Traceback (most recent call last):
File "./testit.py", line 141, in <module>
f1 = getSocketForm(0)
File "./testit.py", line 99, in getSocketForm
return getForm("Socket",n)
File "./testit.py", line 88, in getForm
forms = ClientForm.ParseResponse(response,backwards_compat=False)
File "/usr/lib/python2.5/site-packages/ClientForm.py", line 1057, in ParseResponse
return _ParseFileEx(response, response.geturl(), *args, **kwds)[1:]
File "/usr/lib/python2.5/site-packages/ClientForm.py", line 1128, in _ParseFileEx
type, name, attrs, select_default=select_default, index=ii*10)
File "/usr/lib/python2.5/site-packages/ClientForm.py", line 2843, in new_control
control.add_to_form(self)
File "/usr/lib/python2.5/site-packages/ClientForm.py", line 2016, in add_to_form
Control.add_to_form(self, form)
TypeError: unbound method add_to_form() must be called with FancyControl instance as first argument (got CheckboxControl instance instead)
--
Grant Edwards grante Yow! I want a WESSON OIL
at lease!!
visi.com
Scott David Daniels
is ClientForm apparently defines subclasses of Control such as
CheckboxControl. The monkey-patching only happens after the
ClientForm module has been executed (first import), and the
monkey-patching happens after all of that. So now the "tack it into
the class" method looks a bit better if you cannot simply add your
requirement to the ClientForm source.
--Scott David Daniels
Scott....@Acm.Org
Grant Edwards
>> File "/usr/lib/python2.5/site-packages/ClientForm.py", line 2016, in add_to_form
>> Control.add_to_form(self, form)
>> TypeError: unbound method add_to_form() must be called with FancyControl instance as first argument (got CheckboxControl instance instead)
>
> OK, that is one of the problems with monkey-patching. What happened
> is ClientForm apparently defines subclasses of Control such as
> CheckboxControl.
Right. Control is just a base class, and changing it to a
"different" class after sub-classes have been defined pulls the
rug out from under things. Had I thought about it for a
minute, I should have known that would happen. The working
patch tweaks the class in-place, so it works OK.
> The monkey-patching only happens after the ClientForm module
> has been executed (first import), and the monkey-patching
> happens after all of that. So now the "tack it into the
> class" method looks a bit better if you cannot simply add your
> requirement to the ClientForm source.
That's obviously the "right thing", but it makes portability
more of an issue (I would have to archive/distribute ClientForm
source and users would have to install the customized version
of ClientForm).
Of course there's always the chance that my version of
monkey-patching will stop working with a different version of
ClientForm. We'll burn that bridge when we come to it.
--
Grant Edwards grante Yow! BARBARA STANWYCK makes
at me nervous!!
visi.com
Scott David Daniels
What you might use as a half-way measure:
class Mixin: # or class Mixin(object) if new-style:
def __eq__(self, other):
return (self.type == other.type
and self.name == other.name
and self.value == other.value
and self.disabled == other.disabled
and self.readonly == self.readonly)
def __ne__(self, other):
return not self.__eq__(other)
...(including FancyHTMLForm) ...
class FancyControl(MixIn, ClientForm.Control): pass
class FancyCheckboxControl(MixIn, ClientForm.CheckboxControl): pass
...
ClientForm.HTMLForm = FancyHTMLForm
ClientForm.Control = FancyControl
ClientForm.CheckboxControl = FancyCheckboxControl
--Scott David Daniels
Scott....@Acm.Org
Grant Edwards
That would work -- but there are probably 8 or 10 different
Control subclasses. It's a bit tedious mixing them all one at a
time, and you need more "inside" information (the names of all
the different subclasses).
--
Grant
Gabriel Genellina
escribió:
> On 2009-04-07, Scott David Daniels <Scott....@Acm.Org> wrote:
>> Grant Edwards wrote:
>>> On 2009-04-07, Scott David Daniels <Scott....@Acm.Org> wrote:
>>>
>>>>> File "/usr/lib/python2.5/site-packages/ClientForm.py", line
>>>>> 2016, in add_to_form
>>>>> Control.add_to_form(self, form)
>>>>> TypeError: unbound method add_to_form() must be called with
>>>>> FancyControl instance as first argument (got CheckboxControl
>>>>> instance instead)
>>>
>>>> has been executed (first import), and the monkey-patching
>>>> happens after all of that. So now the "tack it into the
>>>> class" method looks a bit better if you cannot simply add your
>>>> requirement to the ClientForm source.
>>>
>>> That's obviously the "right thing", but it makes portability
>>> more of an issue (I would have to archive/distribute ClientForm
>>> source and users would have to install the customized version
>>> of ClientForm).
>>>
>>> Of course there's always the chance that my version of
>>> monkey-patching will stop working with a different version of
>>> ClientForm. We'll burn that bridge when we come to it.
>>
>> What you might use as a half-way measure:
>>
>> class Mixin: # or class Mixin(object) if new-style:
>> def __eq__(self, other):
>> def __ne__(self, other):
>> return not self.__eq__(other)
>> class FancyCheckboxControl(MixIn, ClientForm.CheckboxControl): pass
>> ..
>> ClientForm.CheckboxControl = FancyCheckboxControl
>
> That would work -- but there are probably 8 or 10 different
> Control subclasses. It's a bit tedious mixing them all one at a
> time, and you need more "inside" information (the names of all
> the different subclasses).
New style classes have a __subclasses__() method that could be used to
find all of them (*at a certain moment*) -- but considering all the
issues, I think that monkey-patching the base class is the "less bad"
option in this case...
--
Gabriel Genellina
Grant Edwards
>>> ClientForm.Control = FancyControl
>>> ClientForm.CheckboxControl = FancyCheckboxControl
>>
>> That would work -- but there are probably 8 or 10 different
>> Control subclasses. It's a bit tedious mixing them all one at a
>> time, and you need more "inside" information (the names of all
>> the different subclasses).
>
> New style classes have a __subclasses__() method that could be
> used to find all of them (*at a certain moment*) -- but
> considering all the issues, I think that monkey-patching the
> base class is the "less bad" option in this case...
Of course I could just
--
Grant Edwards grante Yow! Th' MIND is the Pizza
at Palace of th' SOUL
visi.com
Grant Edwards
>>> class Mixin: # or class Mixin(object) if new-style:
>>> def __eq__(self, other):
>>> return (self.type == other.type ...
>>> def __ne__(self, other):
>>> return not self.__eq__(other)
>>> class FancyControl(MixIn, ClientForm.Control): pass
>>> class FancyCheckboxControl(MixIn, ClientForm.CheckboxControl): pass
>>> ..
>>> ClientForm.Control = FancyControl
>>> ClientForm.CheckboxControl = FancyCheckboxControl
>>
>> That would work -- but there are probably 8 or 10 different
>> Control subclasses. It's a bit tedious mixing them all one at a
>> time, and you need more "inside" information (the names of all
>> the different subclasses).
>
> New style classes have a __subclasses__() method that could be used to
> find all of them (*at a certain moment*) -- but considering all the
> issues, I think that monkey-patching the base class is the "less bad"
> option in this case...
I agree. Inserting methods into the base class seems to be
working fine, and it's the least messy of the alternatives. I'd
submit a patch for ClientForm, but I don't know if my
definition of "equal" for a form/control is generally useful
enough to warrant being added to the package.
FWIW, I'm using it for automated regression-testing on a
product containing a web server. Basically, I perform an
operation on the product, and then check to make sure that only
certain web pages/form/controls changed in the manner they
should have.
--
Grant Edwards grante Yow! ... the HIGHWAY is
at made out of LIME JELLO and
visi.com my HONDA is a barbequeued
OYSTER! Yum!