From: Ævar Arnfjörð Bjarmason Date: Wed, 06 Oct 2010 22:47:45 GMT Subject: Re: [PATCHv5 03/17] gitweb/lib - Very simple file based cache Message-ID: In-Reply-To: On Wed, Oct 6, 2010 at 22:46, Thomas Adam wrote: > On 6 October 2010 23:44, Ævar Arnfjörð Bjarmason wrote: >> On Wed, Oct 6, 2010 at 22:41, Thomas Adam wrote: >>> On 6 October 2010 23:01, Jakub Narebski 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.