Re: [PATCHv5 03/17] gitweb/lib - Very simple file based cache
On Wed, Oct 6, 2010 at 22:46, Thomas Adam <thomas@xteddy.org> wrote:
Show 30 quoted lines
> 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.Ah right, sorry about the noise then.