Perl::Critic and Subroutines

daugh016 has asked for the wisdom of the Perl Monks concerning the following question:

Monks, I have a(some) question(s) having to do with the critique of my subroutines by Perl::Critic. I am new to Perl and have been writing my code to perform what I would like then making the recommendations from Perl::Critic. I know this isn't the most efficient way to learn the language but it has helped somewhat since I am new. Anyway, here are the two sub routines I have:

#!/usr/bin/perl

# $Id: XXX.plx;
# $Revision: 1 $
# $HeadURL: 1 $
# $Source: /Perl/XXX.plx $
# $Date: 10.30.2012 $
# $Author: daugh016 $
use 5.014;    #this enables strict
use warnings;

use vars qw/ $VERSION /;
$VERSION = '1.00';

use English qw(-no_match_vars);
use Readonly;

my $print_err = "Cannot print:\t";

my $var_test = 'This is a test variable';
print_var_with_err( "\$var_test", "\$print_err" );
my @list_test = qw(This is a test list);
print_list_with_err( "\@list_test", "\$print_err" );

# Print variable with error messages
sub print_var_with_err {

    my ( $a, $b ) = @_;
    my $a_eval = eval $a;
    my $b_eval = eval $b;
    print $a . qq{:\n} . $a_eval . qq{\n} or croak( $b_eval . $ERRNO )
+;
    return;
}

# Print list with error messages
sub print_list_with_err {

    my ( $c, $d ) = @_;
    my @c_eval = eval "$c";
    my $d_eval = eval $d;
    while ( my ( $index, $elem ) = each @c_eval ) {
        say $c . q{[} . $index . qq{]:\n} . $elem . qq{\n}
          or croak( $d_eval . $ERRNO );
    }
    return;
}
[download]

The output follows:
$var_test:
This is a test string
@list_test[ 0]:
This

@list_test[ 1]:
is

@list_test[ 2]:
a

@list_test[ 3]:
test

@list_test[ 4]:
list

I am getting the following problems from Perl::Critic:

Useless interpolation of literal string at line 21, column 21. See page 51 of PBP. Severity: 1
Useless interpolation of literal string at line 21, column 35. See page 51 of PBP. Severity: 1
Useless interpolation of literal string at line 23, column 22. See page 51 of PBP. Severity: 1
Useless interpolation of literal string at line 23, column 37. See page 51 of PBP. Severity: 1
Expression form of "eval" at line 29, column 18. See page 161 of PBP. Severity: 5
Expression form of "eval" at line 30, column 18. See page 161 of PBP. Severity: 5
Expression form of "eval" at line 39, column 18. See page 161 of PBP. Severity: 5
Expression form of "eval" at line 40, column 18. See page 161 of PBP. Severity: 5

I cannot figure out how to fix these issues.

Also, since essentially these functions are doing the same thing (one with variables and one with lists), is there a way to combine them where it wouldn't matter if variables or arrays are passed?

Last, I am sure I am missing some other best practices. Any suggestions would be greatly appreciated!

Thank you Monks! You guys/gals are great!

Comment on Perl::Critic and Subroutines Download Code

Replies are listed 'Best First'.
Re: Perl::Critic and Subroutines by davido (Cardinal) on Nov 28, 2012 at 01:08 UTC
I know this isn't the most efficient way to learn the language but it has helped somewhat since I am new. This is hurting you. Efficiency aside, this is the wrong way to learn the language, and will leave you with big gaps in your understanding, inadequacies and kludges in your code, and a cargo-culting approach to programming with Perl. Starting with Perl::Critic and working backwards into a mastery of the language is not going to work. Cease! Desist! :) Start with Learning Perl, from O'Reilly, and then move on to Intermediate Perl, also from O'Reilly. Along the way, read a substantial portion of Perl's POD. There are other resources that would work well too (in lieu of the two O'Reilly books I mentioned), but I know those two will leave you with enough of a foundation in Perl that when you do finally start reading Perl Best Practices (O'Reilly), you'll already know how to program effectively with Perl. Then you will hopefully have the basic knowledge necessary to know when, which, and how to apply the suggestions from Perl Best Practices thoughtfully. Only then, can Perl::Critic be a useful tool. Dave	[reply]
Re: Perl::Critic and Subroutines by tobyink (Canon) on Nov 27, 2012 at 21:53 UTC
'Useless interpolation of literal string' means that you're using double-quoted strings when single-quoted would do. 'Expression form of "eval"' means you're using stringy eval (i.e. `eval "code"`) rather than block eval (`eval { code }`). By the way, what you're doing (passing around names of variables and `eval`ing them) is kinda dumb. Notice how this doesn't work: `sub define_var_and_print_it { my $xyz = "hello"; print_var_with_err("\$xyz", "\$xyz"); }` [download] `perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'`	[reply] [d/l] [select]
Re^2: Perl::Critic and Subroutines by daugh016 (Initiate) on Nov 27, 2012 at 22:26 UTC
For both of the issues, I have tried to do your recommendation but then the functions stopped working. The was the problem I was having-- I cannot implement the recommendations without losing the functionality. Are there better ways to get the same output without the sloppy coding I have done?	[reply]
Re^3: Perl::Critic and Subroutines by tobyink (Canon) on Nov 28, 2012 at 08:17 UTC
"I cannot implement the recommendations without losing the functionality." Well then, Perl::Critic is having the desired effect - steering you away from dodgy areas. :-) "Are there better ways to get the same output without the sloppy coding I have done?" Yes, others have pointed you towards PadWalker. That's a much better way for a sub to peek at its caller's variables. Much better in that it actually usually works. (Your current solution works through luck - because the variables you're peeking at are actually file-level. Here's an example using that. Read more... (2 kB) Better still would be to actually pass the subs the values they need as arguments, so they don't need to peek at their caller's variables... Read more... (1465 Bytes) `perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'`	[reply] [d/l] [select]
Re^4: Perl::Critic and Subroutines by daugh016 (Initiate) on Nov 28, 2012 at 20:46 UTC
Re: Perl::Critic and Subroutines by toolic (Bishop) on Nov 28, 2012 at 03:04 UTC
I cannot figure out how to fix these issues. As another monk pointed out, you can understand more about a policy by reading its POD. Here is a small script I use to make it easier to access the POD: Sort and summarize perlcritic output	[reply]
Re: Perl::Critic and Subroutines by Anonymous Monk on Nov 27, 2012 at 21:57 UTC
I cannot figure out how to fix these issues. Buy the book, then you might stand a chance :) `$ pm_which Perl::Critic C:\perl\site\5.14.1\lib\Perl\Critic.pm $ ack "Useless interpolation of literal string" C:\perl\site\5.14.1\li +b\Perl\Critic C:\perl\site\5.14.1\lib\Perl\Critic\Policy\ValuesAndExpressions\Prohib +itInterpolationOfLiterals.pm 24:Readonly::Scalar my $DESC => q{Useless interpolation of literal str +ing};` [download] perldoc Perl::Critic::Policy::ValuesAndExpressions::ProhibitInterpolationOfLiterals I am not a fan of this policy :) Also, since essentially these functions are doing the same thing (one with variables and one with lists), is there a way to combine them where it wouldn't matter if variables or arrays are passed? What is the purpose of these functions? Your use of string-eval could be critical :)	[reply] [d/l]
Re^2: Perl::Critic and Subroutines by tobyink (Canon) on Nov 28, 2012 at 10:23 UTC
"I am not a fan of this policy :)" There seems little point to it. There is a very slight compile time speed penalty to double-quoted strings. This benchmark demonstrates that: `use Benchmark ':all'; cmpthese(250_000, { e_double => sub { eval q{"foo"} }, e_single => sub { eval q{'foo'} }, });` [download] But at runtime, there's no difference, as they compile to the same optree... `$ perl -MO=Concise -e'$x=q{foo}' > single.txt -e syntax OK $ perl -MO=Concise -e'$x=qq{foo}' > double.txt -e syntax OK $ diff -s single.txt double.txt Files single.txt and double.txt are identical $ rm single.txt double.txt` [download] That is not always the case in all programming languages though. The PHP compiler is less smart than the Perl compiler, so in PHP there's a real speed penalty on double-quoted strings... Read more... (2 kB) `perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'`	[reply] [d/l] [select]
Re^3: Perl::Critic and Subroutines by jthalhammer (Friar) on Dec 04, 2012 at 06:17 UTC
"I am not a fan of this policy :)" There seems little point to it. I actually think this is one of the best policies. As you pointed out, the performance benefits are very small. But it is immensely helpful for clarifying intent and detecting typos. Consider this code: `my $name = <STDIN>; print "Hello, name";` [download] Unless you had test coverage on that (or you just spotted it by eye) you'd never find that bug. But ProhibitUselessInterpolation will find it for you.	[reply] [d/l]
Re^2: Perl::Critic and Subroutines by daugh016 (Initiate) on Nov 27, 2012 at 22:22 UTC
The point of them is that I am tying to avoid having to comment and uncomment multiple lines of print code. Specifically, the list one is 3 lines and I am doing this multiple times with testing. Also, I like to have informative prints where I have the variable/list name and if it is a list , I like to have a label for each element.	[reply]
Re^3: Perl::Critic and Subroutines by Anonymous Monk on Nov 27, 2012 at 23:05 UTC
The point of them is that I am tying to avoid having to comment and uncomment multiple lines of print code ... So you're writing some kind of developer aid, like Smart::Comments? eval probably won't work the way you want -- try PadWalker or Data::Dumper::Names	[reply]
Re^3: Perl::Critic and Subroutines by Anonymous Monk on Nov 27, 2012 at 23:07 UTC
The point of them is that I am tying to avoid having to comment and uncomment multiple lines of print code ... FWIW, that ought to be as simple as calling Ctrl+Q repeatedly (well if you have SciTE, different keyboard shortcuts for different editors)	[reply]