Code review comment for lp:~garyvdm/bzr/register_lazy_decorated

Revision history for this message
Marius Kruger (amanica) wrote :

2009/7/12 Robert Collins <email address hidden>:
> On Sun, 2009-07-12 at 21:08 +0000, Marius Kruger wrote:
...
>> I'd prefer if this doesn't use exception handling to decide
>> what the previous command is, rather check it:
>>
>> -        try:
>> +        if key in self:
>>              previous = self.get(key)
>> -        except KeyError:
>> +        else:
>>              previous = _builtin_commands().get(key
>
> This requires two lookups ('key in self' and 'self.get') vs one. Whats
> the objects to an exception?

Maybe its just some of my java baggage talking, but in general I don't think
its good practice to use exception handling to determine normal code flow.
It can have hidden side effects, like catching unrelated KeyErrors
or have additional exception-creating-overhead, although I do agree
that it is sometimes more efficient just to try something in stead of
checking if you can.
Anyway if you're ok with this, its fine.

« Back to merge proposal