From: Joerg L. <jo...@us...> - 2004-07-27 17:02:34
|
On 27.07.04, Magnus Lie Hetland wrote: [snip] > OK, let's take a look at that as an example... Ok, but as I said, the code in CVS is completely different. Maybe I should have picked another example... > First of all, it uses helper.isstring and helper.isnumber, both of > which use the Leap Before You Look idiom -- I have no quarrels with > them. That is not type checking/type based polymorphism. This is how > it *should* be done <wink> > > The only use of isintance here is: > > if isinstance(l, length): > self.length = l.length > elif helper.isstring(l): > # ... > > Why is this check necessary? Why not use something like this (possibly > wrapped in helper.islength or something, if desired): > > try: > self.length = l.length > except AttributeError: > if helper.isstring(l): > # ... > > Then, if I decided to implement my own length-like class which > supplied the required length attribute (which is, after all, all > that's required) it would work nicely. > > The only thing you achieve by using isintance instead is to close the > door in my face -- "no wannabe lengths permitted". You're right, this should not be the case. Sometimes, however, we introduced some checks such that the user doesn't get hit by some strange error when he calls writeEPSfile. For instance, we check whether the arguments passed to some canvas methods are of a certain type. I don't think this is necessarily bad. See for instance the insert and set method of the canvas._canvas class for two examples. > Such type-based discrimination <wink> is in most cases necessary; if > an object can do the job, just let it. > > (Often, using polymorphism directly, i.e. simply calling a method on > the given object, would be better than using an if statement checking > properties on the object. That's not feasible in special cases such as > this, where you have to deal with numbers and strings and the like, of > course.) Again, you're right - which unfortunately does not mean that the code is perfect in this sense - it developped over quite some time... So if there are some points where you think the present code can be improved in this regard, feel free to point them out (or just send a patch). Jörg |