From: Thomas Adam <thomas@xteddy.org>
Date: Wed, 06 Oct 2010 22:46:29 GMT
Subject: Re: [PATCHv5 03/17] gitweb/lib - Very simple file based cache
Message-ID: <AANLkTi=spE+RGdwz5f4xOTo-ijCNWF9fhZ06aA-tvu7j@mail.gmail.com>
In-Reply-To: <AANLkTik1RzGno+TsZe5OZSU7SeJOhDKWFCZYmrFhH6SZ@mail.gmail.com>

On 6 October 2010 23:44, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Wed, Oct 6, 2010 at 22:41, Thomas Adam <thomas@xteddy.org> wrote:
>> On 6 October 2010 23:01, Jakub Narebski <jnareb@gmail.com> wrote:
>>> +# creates get_depth() and set_depth($depth) etc. methods
>>> +foreach my $i (qw(depth root namespace)) {
>>> +       my $field = $i;
>>> +       no strict 'refs';
>>
>> For each item, you'll set "no strict refs"?  This might be better off
>> outside the loop.  It's still scoped appropriately inside the
>> subroutine.
>>
>>> +       my $file = $self->path_to_key($key);
>>> +       return undef unless (defined $file && -f $file);
>>
>> PBP (Perl Best Practises) will tell you that explicitly returning
>> undef is discouraged -- "undef" should be reserved for those errors
>> you cannot handle, not ones you don't want to.
>
> [...]
>
>>> +       return unless (defined $key && defined $data);
>>
>> return what?
>
> false. You're right that "return undef;" is bad style, but "return;"
> is what you should use instead.

I didn't mean that.  :)  It was a question to ensure this is what was
wanted, and not another undef.  I am well aware of the differences.

-- Thomas Adam