Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Perl-Critic CPAN distribution.

Report information
The Basics
Id: 41633
Status: rejected
Priority: 0/
Queue: Perl-Critic
People
Owner: Nobody in particular
Requestors: EDAVIS [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: 1.092
Fixed in: (no value)

History Show all quoted text
  Thu Dec 11 12:32:19 2008 EDAVIS [...] cpan.org - Ticket created
Subject: Variables::RequireInitializationForLocalVars should not apply to $/
PBP says that when you use 'local' you should explicitly initialize the value. Hence the policy Variables::RequireInitializationForLocalVars. However, PBP (on page 213) explicitly recommends the following idiom: my $code = do { local $/; <$in> }; In a way this is an inconsistency in PBP. But I think the right way to resolve the contradiction is to be generous, and make RequireInitializationForLocalVars tolerate 'local $/'. (The 'local $/' idiom is also recommended in perlfaq5. So I think it is reasonable for it to be treated as a best practice and granted an exemption from the rule that you must always have assignment with 'local'.)
  Thu Dec 11 21:41:56 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #41633] Variables::RequireInitializationForLocalVars should not apply to $/
Date: Thu, 11 Dec 2008 20:41:34 -0600
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
EDAVIS via RT wrote: Show quoted text
> In a way this is an inconsistency in PBP. But I think the right way to > resolve the contradiction is to be generous, and make > RequireInitializationForLocalVars tolerate 'local $/'.
I disagree. Someone new to perl isn't going to understand the difference when they read your code.
  Thu Dec 11 21:41:57 2008 The RT System itself - Status changed from 'new' to 'open'
  Fri Dec 12 15:53:33 2008 thaljef [...] cpan.org - Correspondence added
On Thu Dec 11 12:32:19 2008, EDAVIS wrote: Show quoted text
> So I think it is reasonable for it to be treated as a best practice > and granted an exemption from the rule that you must always have > assignment with 'local'.)
I also disagree, but for a different reason. Wherever possible, I believe that Conway attempts to present each guideline independently. That way, you can understand (and adopt) each guideline without necessarily having to understand (and adopt) other guidelines. The example you cited is in the context of using do{} blocks for file slurping, which is orthogonal to the issue of initializing localized variables. So from that perspective, I believe that the inconsistency is actually deliberate.
  Fri Dec 12 18:43:21 2008 chris+rt [...] chrisdolan.net - Correspondence added
On Thu Dec 11 12:32:19 2008, EDAVIS wrote: Show quoted text
> my $code = do { local $/; <$in> };
I prefer: use File::Slurp qw(read_file); my $content = read_file $filename; which is an order of magnitude more readable. read_file also works on filehandles, but that's somewhat less intuitive. Chris
  Fri Dec 12 21:45:58 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #41633] Variables::RequireInitializationForLocalVars should not apply to $/
Date: Fri, 12 Dec 2008 20:45:44 -0600
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Chris Dolan via RT wrote: Show quoted text
> I prefer: > > use File::Slurp qw(read_file); > my $content = read_file $filename; > > which is an order of magnitude more readable. read_file also works on > filehandles, but that's somewhat less intuitive.
Not to mention that for large files it's a lot faster.
  Sat Dec 13 10:27:59 2008 EDAVIS [...] cpan.org - Correspondence added
I was going to say that File::Slurp doesn't support reading from an existing filehandle, but I see that it does. (I just grepped the manual page for 'filehandle' and found no matches...) Anyway, it's strange that Conway in one part says that the undef-ing behaviour of 'local $x' should be avoided, while elsewhere deliberately relying on it (with a paragraph or two to explain how the 'local $/' idiom works). But if each recommendation is treated as independent, then it's correct for perlcritic to warn on all uses of 'local x', even if it means that recommended 'best practice' code from one part of PBP is then flagged as un-best practice according to a different part. I would suggest adding a new policy to prohibit setting $/ to undef at all (whether with 'local $/' or '$/ = undef') and suggest File::Slurp instead. If I had realized that File::Slurp supports filehandles, that's certainly what I would have used.
  Sun Dec 14 15:52:00 2008 thaljef [...] cpan.org - Status changed from 'open' to 'rejected'