[PATCH 3 of 7 V2] transaction: add onclose/onabort hook for pre-close logic
Gregory Szorc
gregory.szorc at gmail.com
Mon Mar 31 19:24:37 CDT 2014
On 3/31/14, 5:16 PM, Pierre-Yves David wrote:
>
>
> On 03/31/2014 05:14 PM, Gregory Szorc wrote:
>> On 3/31/14, 5:07 PM, Pierre-Yves David wrote:
>>>
>>>
>>> On 03/31/2014 04:19 PM, Durham Goode wrote:
>>>> + if self.count == 1 and self.onclose:
>>>> + self.onclose()
>>>
>>> `onclose is not None`
>>>
>>> Please, otherwise this is going to bit someone in 31.4 months
>>>
>>>> @@ -149,6 +155,9 @@
>>>> self.usages = 0
>>>> self.file.close()
>>>>
>>>> + if self.onabort:
>>>> + self.onabort()
>>>
>>> `onabort is not None`
>>
>> This is an anti-pattern in Python. Unless you are trying to
>> differentiate None from False from [] from {} from '' from any other
>> type whose __nonzero__ can return False, the explicit type comparison
>> can be safely left out.
>
>
> For me, the contrary is the anti pattern. it is very easy to slip
> variable type to something that may exist and still be false.
>
> I've lost multiple days in the last 5 years because of people not using
> `is not None`
>
> (explicit is better than implicit)
I don't see how that applies here as you are calling self.onclose and
self.onabort immediately after the check. That will raise if the thing
isn't callable.
More information about the Mercurial-devel
mailing list