Re: [PATCHv5 03/17] gitweb/lib - Very simple file based cache

On Wed, Oct 6, 2010 at 22:41, Thomas Adam [off-list ref] wrote:

On 6 October 2010 23:01, Jakub Narebski [off-list ref] wrote:

quoted

+# 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.

quoted

+       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.

[...]

quoted

+       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.

Then you'll get undef in scalar context, and an empty list in list
context. With "return undef" you'll always get an undef, so:

    sub blah { retur undef }
    my (@foo) = blah();

Will make @foo = (undef), which'll make it evaluate to true in scalar
context since there's an item in the array, and give you a useless
array item.