Re: [PATCH 3/5] gitweb: Return plain booleans in validation methods

From: Jakub Narębski <jnareb@gmail.com>
Date: Dec 5, 2013, 19:11 UTC
Message-ID: <CANQwDwdw6MQW2wWvL=GmwYeDKF-gikNx55QdYH2oD50ZZUcPRw@mail.gmail.com>
In-Reply-To: <xmqq61r3i2ol.fsf@gitster.dls.corp.google.com>

On Thu, Dec 5, 2013 at 7:16 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Krzesimir Nowak <krzesimir@endocode.com> writes:
>> On Wed, 2013-12-04 at 17:07 +0100, Jakub Narębski wrote:

Show 17 quoted lines

>>> The only change that needs to be done is replacing
>>>
>>>            return $input;
>>>
>>> with
>>>
>>>            return 1;
>>>
>>
>> I prefer to use zeros instead of undefs - one might wonder if that undef
>> is somewhat special that we can't use 0.
>
> For Perl speakers, I suspect the code gives a totally opposite
> impression.  Normal "false" return from a sub, when there is no
> special need, is to return an undef from it, and a "return 0" would
> make the readers wonder if there is something special about the way
> the returned value has to be numeric zero, no?

Or even plain "return;" (see explanation for policy in [1])... though for functions returning scalar it is recommended to use explicit "return 0;" (or "return undef;").

Anyway, it is easier to see the change and intention of the change if all that is changed id "return $input;" to "return 1;"

But I am not against "return 0;" on validation error (would putting it in separate patch make review easier, or just pointlessly proliferate patches?).

[1]: http://search.cpan.org/~thaljef/Perl-Critic-1.121/lib/Perl/Critic/Policy/Subroutines/ProhibitExplicitReturnUndef.pm

-- 
Jakub Narebski