Code review comment for lp:~jml/launchpad/package-permission-love

Revision history for this message
Jonathan Lange (jml) wrote :

On Tue, Sep 1, 2009 at 9:24 PM, Julian
Edwards<email address hidden> wrote:
> On Monday 31 August 2009 03:51:07 Jonathan Lange wrote:
>> > Whenever I see exceptions used, I ask myself "is this caused by an
>> > exceptional circumstance?"  In this case, I'm not sure it is, but I can
>> > see why you've used one - so that you can return an error string as well.
>> >  Is that a good enough reason to use an exception here?  I guess the
>> > code's neater but I still have a nagging feeling that it's bad form.
>> >
>> > How does this look in comparison:
>> >
>> >    error = verify_upload(
>> >        signer, source_name, archive, self.changes.dsc.component,
>> >        not self.is_new)
>> >    if error is not None:
>> >        self.reject(error)
>>
>> I've never really understood the reasoning behind the "exceptional
>> circumstance" argument.
>
> Exceptions should be thrown only when there is an abnormal condition that the
> code cannot handle.  Exceptions should convey: "Woa!  Something went wrong
> here and I can't deal with it.  BOOM."  In this case, you're using them as a
> routine return value, which doesn't fit that requirement.

That's just restating your point in different words. I really want to
understand the _reason_ for saying so.

> Also bearing in
> mind that they're pretty expensive, if a piece of code can return a value to
> indicate its results, then it should do so.
>

Are they really that expensive in Python?
http://www.python.org/doc/faq/general/#how-fast-are-exceptions is
unclear

>> I also love deleting unneeded tests.
>
> And correcting my grammar ;)
>

Oh, I didn't notice. Sorry :)

jml

« Back to merge proposal