Open Stack

On Thu, Jul 11, 2013 at 10:50 AM, Mark McLoughlin <markmc at redhat.com> wrote:

> On Wed, 2013-07-10 at 21:14 +0200, Thomas Hervé wrote:
> >
> >
> > On Wed, Jul 10, 2013 at 8:32 PM, Mark McLoughlin <markmc at redhat.com>
> > wrote:
> >         On Wed, 2013-07-10 at 11:01 -0700, Nachi Ueno wrote:
> >
> >         > Personally, I prefer not to use exception for such cases.
> >
> >
> >
> > The key here is "personally". I don't think we have to agree on all
> > style issues.
>
> When it results in a patch submitter getting a -1 from one person for
> choosing EAFP and a -1 from another person for choosing LBYL, then
> yes ... actually we do need to agree.
>
>
I feel there is no perfect answer here, it depends on the particular piece
of code. Reviews should mostly focus on the overall model and logic, not
whether 2 lines should be written one way or another. This is blatant
bikeshedding.

>
> > hasattr is a bit dangerous as it catches more exceptions than it needs
> > too. See for example
> >
> http://stackoverflow.com/questions/903130/hasattr-vs-try-except-block-to-deal-with-non-existent-attributes/16186050#16186050for an explanation.
>
> That answer does begin with this, though:
>
>   I almost always use hasattr: it's the correct choice for most cases.
>
> and, frankly, a __getattr__() method that returns ValueError is broken.
>
> i.e. the conclusion would be that we should only avoid hasattr() in some
> very limited cases where the underlying __getattr__() does weird things
> or where using it can result in a race condition.
>

Personally I'd rather use something that works 100% of the time than
something that only works 99.9% of the time. I agree that a __getattr__
raising ValueErro is broken, but if you're already using hasattr you'll
only know until it's too late.

Also never have attributes conditionally it's terrible :).

Regards,

-- 
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130711/7d239870/attachment.html>