Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-19 Thread Aldo Calpini

Ovid wrote:

Readonly constants are just easier to use and have fewer gotchas.


they have indeed, when you need to access the constants from outside of 
the module they are declared in (which is a pretty common case).


cfr. Foo::Bar::CONSTANT_FIELD vs. $Foo::Bar::CONSTANT_FEILD. the latter 
fools strict.


IMHO, the unconditional sponsoring of Readonly by PBP is just plain wrong.


cheers,
Aldo



Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-19 Thread Ovid
- Original Message 

 From: Aldo Calpini d...@perl.it

 Ovid wrote:
  Readonly constants are just easier to use and have fewer gotchas.
 
 they have indeed, when you need to access the constants from outside of the 
 module they are declared in (which is a pretty common case).

It also used to be very common not to use strict or warnings.  This doesn't 
mean it's a good thing.  The module in question should provide a sub or method 
to provide access to this data.  Java programmers learned long ago not to let 
people tough their privates, Perl programmers should learn the same thing.  
(For example, when the constant is computed rather than declared, wrapping it 
in a sub saves a *lot* of grief -- if you've done that up front and not forced 
people to change their APIs).

If you *must* use globals, it's certainly a good thing that they're read-only 
but at the very least, do it sanely:

  package Some::Module;
  use Readonly;
  Readonly our $foo = 3;
  use Exporter 'import'
  our @EXPORT_OK = ('$foo'):


Note that we've been forced to switch from 'my' to 'our' in the Readonly 
variable, but now the calling code doesn't need a fully-qualified package name:

  use Some::Module '$foo';
  print $foo;

Cheers,
Ovid
--
Buy the book - http://www.oreilly.com/catalog/perlhks/
Tech blog- http://use.perl.org/~Ovid/journal/
Twitter  - http://twitter.com/OvidPerl
Official Perl 6 Wiki - http://www.perlfoundation.org/perl6



Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-19 Thread David Cantrell
On Thu, Feb 19, 2009 at 09:04:35AM +0100, Aldo Calpini wrote:

 IMHO, the unconditional sponsoring of Readonly by PBP is just plain wrong.

An awful lot of PBP is Just Plain Wrong if you treat it as hard-and-
fast rules that should be obeyed all the time.  Thankfully, the book
makes it clear that it shouldn't be treated that way.

-- 
David Cantrell | http://www.cantrell.org.uk/david

  engineer: n. one who, regardless of how much effort he puts in
to a job, will never satisfy either the suits or the scientists


RE: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-19 Thread Roger Hall
That is primarily due to their special ability to slay powerful beasties. :}

Roger

-Original Message-
From: Aristotle Pagaltzis [mailto:pagalt...@gmx.de] 
Sent: Thursday, February 19, 2009 10:13 AM
To: module-authors@perl.org
Subject: Re: ARGH! (was FW: Perl Critic and (honest) hash references)

 Everyone seems to be looking for silver bullets.





RE: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Roger Hall
Bill,

We can exclude specific policies in the code with specially formatted
comments. The M in this (RTFM) case is here: 

http://search.cpan.org/~elliotjs/Perl-Critic-1.096/lib/Perl/Critic.pm#BENDIN
G_THE_RULES

The specific policies aren't properly linked, but Google knows all:

http://search.cpan.org/~elliotjs/Perl-Critic-1.082/lib/Perl/Critic/PolicySum
mary.pod


I haven't actually figured out which policy to exclude yet though. 

Roger


-Original Message-
From: Bill Ward [mailto:b...@wards.net] 
Sent: Wednesday, February 18, 2009 11:11 AM
To: raha...@ualr.edu
Subject: Re: ARGH! (was FW: Perl Critic and (honest) hash references)

What was the solution?

On Wed, Feb 18, 2009 at 8:53 AM, Roger Hall raha...@ualr.edu wrote:
 RTFM is always pretty good advice, eh? :}

 -Original Message-
 From: Roger Hall [mailto:raha...@ualr.edu]
 Sent: Wednesday, February 18, 2009 10:05 AM
 To: module-authors@perl.org
 Subject: Perl Critic and (honest) hash references


 $config-{query}


 Perlcritic complains that Private Member Data shouldn't be accessed
 directly because Accessing an objects data directly breaks encapsulation
 and should be avoided. I get that. Only problem: it's not an object. It's
 just a hashref.




RE: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Roger Hall
I had to dig around in the policy modules because it isn't actually listed
in the other document I linked.

Specifically: ProhibitAccessOfPrivateData

I'm only sure this is it because the error message that came out of the
report ... 

Private Member Data shouldn't be accessed directly at line X, column Y.
Accessing an objects data directly breaks encapsulation and should be
avoided.

... is prominently displayed in the module.

Thanks!

Roger 

-Original Message-
From: Bill Ward [mailto:b...@wards.net] 
Sent: Wednesday, February 18, 2009 11:11 AM
To: raha...@ualr.edu
Subject: Re: ARGH! (was FW: Perl Critic and (honest) hash references)

What was the solution?

On Wed, Feb 18, 2009 at 8:53 AM, Roger Hall raha...@ualr.edu wrote:
 RTFM is always pretty good advice, eh? :}

 -Original Message-
 From: Roger Hall [mailto:raha...@ualr.edu]
 Sent: Wednesday, February 18, 2009 10:05 AM
 To: module-authors@perl.org
 Subject: Perl Critic and (honest) hash references

 $config-{query}

 Perlcritic complains that Private Member Data shouldn't be accessed
 directly because Accessing an objects data directly breaks encapsulation
 and should be avoided. I get that. Only problem: it's not an object. It's
 just a hashref.




Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Jonas Brømsø Nielsen

Hi Roger,

How do you perform your perlcritic runs?

I can recommend the verbosity setting 8

perlcritic --verbose 8

This gives you quite friendly policy identifiers

	[ValuesAndExpressions::ProhibitConstantPragma] Pragma constant used  
at line 22, column 1. (Severity: 4)


well my favorite anyway

jonasbn

On 18/02/2009, at 21.17, Roger Hall wrote:

I had to dig around in the policy modules because it isn't actually  
listed

in the other document I linked.

Specifically: ProhibitAccessOfPrivateData

I'm only sure this is it because the error message that came out of  
the

report ...

Private Member Data shouldn't be accessed directly at line X,  
column Y.

Accessing an objects data directly breaks encapsulation and should be
avoided.

... is prominently displayed in the module.

Thanks!

Roger

-Original Message-
From: Bill Ward [mailto:b...@wards.net]
Sent: Wednesday, February 18, 2009 11:11 AM
To: raha...@ualr.edu
Subject: Re: ARGH! (was FW: Perl Critic and (honest) hash references)

What was the solution?

On Wed, Feb 18, 2009 at 8:53 AM, Roger Hall raha...@ualr.edu wrote:

RTFM is always pretty good advice, eh? :}

-Original Message-
From: Roger Hall [mailto:raha...@ualr.edu]
Sent: Wednesday, February 18, 2009 10:05 AM
To: module-authors@perl.org
Subject: Perl Critic and (honest) hash references

$config-{query}

Perlcritic complains that Private Member Data shouldn't be accessed
directly because Accessing an objects data directly breaks  
encapsulation
and should be avoided. I get that. Only problem: it's not an  
object. It's

just a hashref.







RE: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Roger Hall
Jonas,

Honestly I just left the default perlcritic test script in my package as
generated by Module::Starter. This was the first time I had done so, and I
really had no idea about Perl::Critic until last night when my module failed
smoke testing after upload to CPAN. From the test script I am guessing
that I cannot alter the verbosity setting (running on someone else's
automated server).


#!perl

if (!require Test::Perl::Critic) {
Test::More::plan(
skip_all = Test::Perl::Critic required for testing PBP compliance
);
}

Test::Perl::Critic::all_critic_ok();


Incidentally, I have tried to install P::C today, but I can't get past PPI
(or a dependency). I keep running out of memory (which may be a failing
memory stick in my machine, because it has 8 gigs of RAM). Someday maybe
I'll try again and kill the test that is sticking: 

t/14_charsetsok 1/11Out of memory!

It is from: ADAMK/PPI-1.203.tar.gz

Thanks for the suggestion!

Roger


-Original Message-
From: Jonas Brømsø Nielsen [mailto:jona...@gmail.com] 
Sent: Wednesday, February 18, 2009 2:23 PM
To: raha...@ualr.edu
Cc: 'Bill Ward'; module-authors@perl.org
Subject: Re: ARGH! (was FW: Perl Critic and (honest) hash references)

Hi Roger,

How do you perform your perlcritic runs?

I can recommend the verbosity setting 8

perlcritic --verbose 8

This gives you quite friendly policy identifiers

[ValuesAndExpressions::ProhibitConstantPragma] Pragma constant
used  
at line 22, column 1. (Severity: 4)

well my favorite anyway

jonasbn

On 18/02/2009, at 21.17, Roger Hall wrote:

 I had to dig around in the policy modules because it isn't actually  
 listed
 in the other document I linked.

 Specifically: ProhibitAccessOfPrivateData

 I'm only sure this is it because the error message that came out of  
 the
 report ...

 Private Member Data shouldn't be accessed directly at line X,  
 column Y.
 Accessing an objects data directly breaks encapsulation and should be
 avoided.

 ... is prominently displayed in the module.

 Thanks!

 Roger

 -Original Message-
 From: Bill Ward [mailto:b...@wards.net]
 Sent: Wednesday, February 18, 2009 11:11 AM
 To: raha...@ualr.edu
 Subject: Re: ARGH! (was FW: Perl Critic and (honest) hash references)

 What was the solution?

 On Wed, Feb 18, 2009 at 8:53 AM, Roger Hall raha...@ualr.edu wrote:
 RTFM is always pretty good advice, eh? :}

 -Original Message-
 From: Roger Hall [mailto:raha...@ualr.edu]
 Sent: Wednesday, February 18, 2009 10:05 AM
 To: module-authors@perl.org
 Subject: Perl Critic and (honest) hash references

 $config-{query}

 Perlcritic complains that Private Member Data shouldn't be accessed
 directly because Accessing an objects data directly breaks  
 encapsulation
 and should be avoided. I get that. Only problem: it's not an  
 object. It's
 just a hashref.





RE: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Chris Dolan
 Honestly I just left the default perlcritic test script in my package as
 generated by Module::Starter. This was the first time I had done so, and I
 really had no idea about Perl::Critic until last night when my module
 failed
 smoke testing after upload to CPAN. From the test script I am guessing
 that I cannot alter the verbosity setting (running on someone else's
 automated server).

Score one for the good guys: annoying our users into writing better code. 
;-)

Chris



RE: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Roger Hall
Or at least figuring out how to turn it off! ;}

(Actually, I also had a bare file handle and two-arg file open; *that* I
changed. So I am proud for the help!)

BTW - could you change the smoke settings for perlcritic to verbose as Jonas
described? (Or do I misunderstand how the smoke system works?)

Roger

-Original Message-
From: Chris Dolan [mailto:ch...@chrisdolan.net] 
Sent: Wednesday, February 18, 2009 2:44 PM
To: raha...@ualr.edu
Cc: 'Jonas Brømsø Nielsen'; module-authors@perl.org
Subject: RE: ARGH! (was FW: Perl Critic and (honest) hash references)

 Honestly I just left the default perlcritic test script in my package as
 generated by Module::Starter. This was the first time I had done so, and I
 really had no idea about Perl::Critic until last night when my module
 failed
 smoke testing after upload to CPAN. From the test script I am guessing
 that I cannot alter the verbosity setting (running on someone else's
 automated server).

Score one for the good guys: annoying our users into writing better code. 
;-)

Chris



Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Bill Ward
Still, that's bogus for ordinary hashes... it should only care about that
for objects.  Though I wonder how it could possibly know the difference.

On Wed, Feb 18, 2009 at 12:17 PM, Roger Hall raha...@ualr.edu wrote:

 I had to dig around in the policy modules because it isn't actually listed
 in the other document I linked.

 Specifically: ProhibitAccessOfPrivateData

 I'm only sure this is it because the error message that came out of the
 report ...

 Private Member Data shouldn't be accessed directly at line X, column Y.
 Accessing an objects data directly breaks encapsulation and should be
 avoided.

 ... is prominently displayed in the module.

 Thanks!

 Roger

 -Original Message-
 From: Bill Ward [mailto:b...@wards.net]
 Sent: Wednesday, February 18, 2009 11:11 AM
 To: raha...@ualr.edu
 Subject: Re: ARGH! (was FW: Perl Critic and (honest) hash references)

 What was the solution?

 On Wed, Feb 18, 2009 at 8:53 AM, Roger Hall raha...@ualr.edu wrote:
  RTFM is always pretty good advice, eh? :}
 
  -Original Message-
  From: Roger Hall [mailto:raha...@ualr.edu]
  Sent: Wednesday, February 18, 2009 10:05 AM
  To: module-authors@perl.org
  Subject: Perl Critic and (honest) hash references
 
  $config-{query}
 
  Perlcritic complains that Private Member Data shouldn't be accessed
  directly because Accessing an objects data directly breaks
 encapsulation
  and should be avoided. I get that. Only problem: it's not an object.
 It's
  just a hashref.





Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Bill Ward
On Wed, Feb 18, 2009 at 12:22 PM, Jonas Brømsø Nielsen jona...@gmail.comwrote:

 Hi Roger,

 How do you perform your perlcritic runs?

 I can recommend the verbosity setting 8

perlcritic --verbose 8

 This gives you quite friendly policy identifiers

[ValuesAndExpressions::ProhibitConstantPragma] Pragma constant
 used at line 22, column 1. (Severity: 4)


What's wrong with 'use constant'?


Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Ovid
 From: Bill Ward b...@wards.net

 This gives you quite friendly policy identifiers

   [ValuesAndExpressions::ProhibitConstantPragma] Pragma constant used 
 at line 22, column 1. (Severity: 4)

 What's wrong with 'use constant'?

Well, nothing's wrong with it.  It does, however, get clumsy in some cases.  
For example:

  use constant FOO = 3;
  print $data-{+FOO}; 
  print We have @{[FOO]} widgets;
  # or
  print We have .FOO. widgets;

Versus:

  use Readonly;
  Readonly my $FOO = 3;
  print $data-{$FOO}; 
  print We have $FOO widgets;

Readonly constants are just easier to use and have fewer gotchas.

Cheers,
Ovid
--
Buy the book - http://www.oreilly.com/catalog/perlhks/
Tech blog- http://use.perl.org/~Ovid/journal/
Twitter  - http://twitter.com/OvidPerl
Official Perl 6 Wiki - http://www.perlfoundation.org/perl6


Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Ezra Cooper

On Feb 18, 2009, at 9:08 PM, Bill Ward wrote:

Still, that's bogus for ordinary hashes... it should only care  
about that for objects.  Though I wonder how it could possibly know  
the difference.


Can we define an object as a blessed hash reference? And leave  
unblessed hashes available as little bundles of data?


We could distinguish between objects which are 100% encapsulated  
and records which are 0% encapsulated--just handy slates of public  
data.


2¢,
Ezra



Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Curtis Jewell

On Wed, 18 Feb 2009 22:03 +, Ezra Cooper e...@ezrakilty.net
wrote:
 On Feb 18, 2009, at 9:08 PM, Bill Ward wrote:
 
  Still, that's bogus for ordinary hashes... it should only care  
  about that for objects.  Though I wonder how it could possibly know  
  the difference.

Only by executing the program, I think.  It couldn't be a static test.

--
Curtis Jewell
swords...@csjewell.fastmail.us

%DCL-E-MEM-BAD, bad memory
-VMS-F-PDGERS, pudding between the ears

[I use PC-Alpine, which deliberately does not display colors and
pictures in HTML mail]

--
Curtis Jewell
swords...@csjewell.fastmail.us

%DCL-E-MEM-BAD, bad memory
-VMS-F-PDGERS, pudding between the ears

[I use PC-Alpine, which deliberately does not display colors and pictures in 
HTML mail]



Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Bill Ward
On Wed, Feb 18, 2009 at 3:35 PM, Curtis Jewell 
perl.module-auth...@csjewell.fastmail.us wrote:


 On Wed, 18 Feb 2009 22:03 +, Ezra Cooper e...@ezrakilty.net
 wrote:
  On Feb 18, 2009, at 9:08 PM, Bill Ward wrote:
 
   Still, that's bogus for ordinary hashes... it should only care
   about that for objects.  Though I wonder how it could possibly know
   the difference.

 Only by executing the program, I think.  It couldn't be a static test.


Yeah, I agree.  I can see it as a warning, but not as a hard error.

I've never used Perl::Critic actually, but we're looking into using it to
supplement our existing (homemade) standards compliance checker at work.


Re: ARGH! (was FW: Perl Critic and (honest) hash references)

2009-02-18 Thread Elliot Shank

This was caused by the tester having Perl::Critic::Nits installed, which is not 
part of core Perl::Critic.

Perl::Critic tests should NOT be enabled by default for any CPAN distribution.  
Do with your P::C test whatever you do with the rest of your author tests to 
prevent them running by default.

As others have said, you can specify the verbosity in a perlcriticrc file.  
Personally, I use this everywhere:

   verbose = %f: %m at line %l, column %c.  %e.  (Severity: %s, %p)\n

This includes the short name of the policy (the %p) at the end.

You could have blocked this policy and all other non-core policies using the 
theme option:

   theme = core

But you should still shouldn't allow it to run by default.  Let us say that 
your code is perfect as far as P::C is concerned.  Your distribution gets 
installed on thousands of peoples' machines and all the tests run perfectly on 
every single one of them.  A year goes by and everybody is happy (without you 
having to change a thing).  A new version of Perl::Critic comes out with a new 
policy which your code doesn't comply with (you evil person, your POD isn't 
written in Pig Latin!).  Suddenly, your code looks bad and no one can install 
your distribution.

You really don't want Perl::Critic being run as part of regular testing in a 
CPAN distribution.  (DarkPAN code is another matter.)