[PATCH 1 of 2] extensions: add get contextmanager to avoid common pattern
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Sun Jan 17 13:57:58 CST 2016
On 01/15/2016 07:14 AM, Yuya Nishihara wrote:
> On Thu, 14 Jan 2016 14:45:47 -0800, Gregory Szorc wrote:
>> On Thu, Jan 14, 2016 at 2:17 PM, Laurent Charignon <lcharignon at fb.com>
>>> # HG changeset patch
>>> # User Laurent Charignon <lcharignon at fb.com>
>>> # Date 1452809804 28800
>>> # Thu Jan 14 14:16:44 2016 -0800
>>> # Node ID c1ba4521aea0906874fe7de5bee156d5070c0e56
>>> # Parent e6e34c4e391668c5d8af8f98c004a27c77b1e2fa
>>> extensions: add get contextmanager to avoid common pattern
>>>
>>> Before this patch, people were sometimes using the following pattern:
>>>
>>> try:
>>> m = extensions.find(name)
>>> ...
>>> except KeyError:
>>> pass
>>>
>>> This patch simplifies the pattern with context managers as follow:
>>>
>>> with extensions.get(name) as m:
>>> ...
>
>> Furthermore, context managers don't feel like the right fit here. Generic
>> replacement for many try..finally, yes. For try..except, not really.
>
> I agree with Gregory. Because it had no cleanup process, using context manager
> would just introduce extra nested block and prevent early return.
>
> If you think KeyError is ugly, how about making it return None?
>
> m = extensions.get(name)
> if not m:
> return
I'm +1 for a None version.
In my opinion, standard Code flow based on KeyError should be avoided
when possible.
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list