Re: [PATCH] gitweb: merge boolean feature subroutines

[demerphq <demerphq@gmail.com>]

2009/1/4 Junio C Hamano <gitster@pobox.com>:
> demerphq <demerphq@gmail.com> writes:
>
>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
>> [...]
>>> -sub feature_blame {
>>> -       my ($val) = git_get_project_config('blame', '--bool');
>>> +sub feature_bool {
>>> +       my $key = shift;
>>> +       my ($val) = git_get_project_config($key, '--bool');
>>>
>>>        if ($val eq 'true') {
>>>                return 1;
>>
>> Maybe that should be:
>>
>>            return ($val eq 'true');
>>
>> as It is not a good idea to use 0 as a replacement for perls false, as
>> the two have different behaviour.
>
> I'd rather want to keep our scripts free of deep Perl magic.  Even if
> there are SVs that are interpreted as false other than 0, that does not
> necessarily mean you have to fear that 0 can sometimes evaluate to true.

No, thats not the point. The point is that why should the code do more
work to return a value that a perl programmer might find unexpected.
Especially when the function has the word "bool" in it. Its like
writing a function whose name says "int" that returns a double. If the
routine was not called "bool" then it wouldnt matter at all.

> As long as you refrain from doing something crazy like "0 but true",
> people who are not (and/or are not inclined to become) familiar with the
> gory innards of Perl can rely on 0 being false and 1 being true when
> calling feature_something subs, no?

Why execute more opcodes, and return a slightly surprising false, when
you dont have to?

Is it really deep perl magic to do:

  return $val eq 'true';

instead of

  return $val eq 'true' ? 1 : 0;

or the actual use:

   if ($val eq 'true') {
      return 1
   } else {
      return 0
   }

Isn't the former superior just on pure minimalism metrics? Theres less
code to understand, less code to go wrong, and as a bonus it returns a
true boolean. Isn't that just a win-win-win? I mean most perl
programmers I know would instantly convert the latter two to the first
just on the grounds that the first version is the clearest expression
of the desired intent.

Anyway, leave it or not, its a minor nit.

cheers,
Yves




-- 
perl -Mre=debug -e "/just|another|perl|hacker/"