[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