Re: [Zope-Coders] Re: Question about procedures
[moved from -coders to -dev] First off: I will go with Lennart's suggestion of branches; I fundamentally agree; I have just never worked on a project where branches for such petty things aren't overkill. I guess Zope is a number of magnitudes larger. :) Now sorry for the noise on zope-coders when this should have been on zope-dev in the first place. Now for some of the things that Florent pointed out: also sprach Florent Guillaume <fg@nuxeo.com> [2005.03.24.1814 +0100]:
- if RESPONSE is not None: + if RESPONSE is not None and ob:
You should check 'and ob is not None' too.
... but ob is false when it is None, no?
But why could it be None ? What's the point (sorry I don't have context).
Well, I was trying to guard against errors made in other parts of the code. I know I should not do this. It made things a lot easier while I was preparing some other patches. Anyway, good thing I haven't committed. :)
+ if not hasattr(ob, 'absolute_url'):
Do not use hasattr for persistent objects. Use if getattr(ob, 'absolute_url', None) is None:
Can I read up on the rationale somewhere?
+ raise TypeError('constructInstance did not return a CMF object.')
Also, check your indentation (should be 4 chars).
Woops.
- return ob.getId() + return getattr(ob, 'id', None)
Please don't do that, getId() is the proper API to call.
Another instance of "what to do when ob does not have an getId method".. you are right, this is wrong. also sprach Andreas Jung <lists@andreas-jung.com> [2005.03.25.0945 +0100]:
For changes which are limited to a file or a subtree I do always prefer a patch instead of a branch.
I can create a branch and submit patches to you (this is when I wish zope.org would be using GNU arch). Anyway, since it's probably best for me not to make changes in the code at present time (being young in the project and without an assigned field of responsibility), where do I send potential patches? This list? -- martin; (greetings from the heart of the sun.) \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck invalid/expired pgp subkeys? use subkeys.pgp.net as keyserver! spamtraps: madduck.bogus@madduck.net "next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank god for the better sleep he enjoys after this process of grotesque self-deception." -- mark twain
martin f krafft <madduck@madduck.net> wrote:
also sprach Florent Guillaume <fg@nuxeo.com> [2005.03.24.1814 +0100]:
- if RESPONSE is not None: + if RESPONSE is not None and ob:
You should check 'and ob is not None' too.
... but ob is false when it is None, no?
Yes but comparing to None is faster, and in some cases (REQUEST for instance), much much faster, than checking the boolean value.
But why could it be None ? What's the point (sorry I don't have context).
Well, I was trying to guard against errors made in other parts of the code. I know I should not do this. It made things a lot easier while I was preparing some other patches. Anyway, good thing I haven't committed. :)
+ if not hasattr(ob, 'absolute_url'):
Do not use hasattr for persistent objects. Use if getattr(ob, 'absolute_url', None) is None:
Can I read up on the rationale somewhere?
It was discussed at length on the zope lists. Basically hasattr tries to access the attribute and returns false if *any* exception is raised. This hides exceptions, which is bad in the case of persistent objects that can raise ConflictErrors. Florent -- Florent Guillaume, Nuxeo (Paris, France) CTO, Director of R&D +33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com
Em Ter, 2005-03-29 às 17:21 +0200, Florent Guillaume escreveu:
martin f krafft <madduck@madduck.net> wrote:
also sprach Florent Guillaume <fg@nuxeo.com> [2005.03.24.1814 +0100]:
- if RESPONSE is not None: + if RESPONSE is not None and ob:
You should check 'and ob is not None' too.
... but ob is false when it is None, no?
Yes but comparing to None is faster, and in some cases (REQUEST for instance), much much faster, than checking the boolean value.
And not every "False" object is None. A custom object could implement __len__() and be considered false if it's "empty", or it could implement __nonzero__() and be considered "false" even when you want to return it. Cheers, Leo -- Leonardo Rochael Almeida <leo@hiper.com.br>
martin f krafft wrote:
Do not use hasattr for persistent objects. Use if getattr(ob, 'absolute_url', None) is None:
Can I read up on the rationale somewhere?
hasattr catches all exceptions and returns false. Now google for COnflictError ;-)
+ raise TypeError('constructInstance did not return a CMF object.')
Also, check your indentation (should be 4 chars).
Woops.
You should probably run the unit tests before you commit anything. That also means you should write tests to excercise the changes you're making, and make sure the tests fail BEFORE you try and fix anything.
For changes which are limited to a file or a subtree I do always prefer a patch instead of a branch.
I can create a branch and submit patches to you (this is when I wish zope.org would be using GNU arch). Anyway, since it's probably best for me not to make changes in the code at present time (being young in the project and without an assigned field of responsibility), where do I send potential patches? This list?
Patches should go in collector entries. cheers, Chris -- Simplistix - Content Management, Zope & Python Consulting - http://www.simplistix.co.uk
also sprach Chris Withers <chris@simplistix.co.uk> [2005.03.30.1513 +0200]:
You should probably run the unit tests before you commit anything. That also means you should write tests to excercise the changes you're making, and make sure the tests fail BEFORE you try and fix anything.
Okay. Man, this is fun. :) -- martin; (greetings from the heart of the sun.) \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck invalid/expired pgp subkeys? use subkeys.pgp.net as keyserver! spamtraps: madduck.bogus@madduck.net eleventh law of acoustics: in a minimum-phase system there is an inextricable link between frequency response, phase response and transient response, as they are all merely transforms of one another. this combined with minimalization of open-loop errors in output amplifiers and correct compensation for non-linear passive crossover network loading can lead to a significant decrease in system resolution lost. however, of course, this all means jack when you listen to pink floyd.
participants (4)
-
Chris Withers -
Florent Guillaume -
Leonardo Rochael Almeida -
martin f krafft