Perl::Critic - new release

So a new release of Perl::Critic was released, and all I want to say is what a faff.

Some key new features:

1) You must check the return value of all eval statement - don't rely on $EVAL_ERROR/$@

Now, this is a good thing(tm) but it does have some pitfalls, such as where you might be evalling a transactional commit.

eval {
$transaction_state and $dbh->commit();
} or do {
...some 'croak' code
};

The problem here is that the eval isn't a croak if $transaction_state is 0 (as it could be inside a much larger transaction), but the return code would be 0, therefore firing off your 'croak' code.

So, to get round this, you need to return a true value after the statement;

eval {
$transaction_state and $dbh->commit();
1;
} or do {
...some 'croak' code
};

A bit off a faff, but it is better code for it.

2) Declaration of all numbers other than 0,1 and 2

So, as it could be difficult to understand what a number represents when reading the code, you now need to use Readonly to declare any numbers with a named variable at the top of your code.

So now instead of

$percentage = $x * 100/$y;

You need

use Readonly;
Readonly our $PERCENTAGE_MAKER => 100;
...
$percentage = $x * $PERCENTAGE_MAKER/$y;
...

This 'might' make sense to odd numbers floating around, but it also applies to indices on arrays. So if you want the 5th element on an array intstead of requesting

$wanted = $array[4];

You now need

use Readonly;
Readonly our $FIFTH_ARRAY_ELEMENT => 4;
...
$wanted = $array[$FIFTH_ARRAY_ELEMENT];
...

Now, I admit it is often bad to pick out many individual elements from an array by specific number, but it will seriously clutter code where this may be necessary. I admit, for a couple of lines in my code, I have now used ##no critic at the end.

I also wonder why specifically 0,1 and 2 get let off. If the problem is that you don't know if 60 means

i) minutes in an hour
ii) seconds in a minute
iii) degrees in the angle of an equilateral triangle

then 0 could mean off, false, nothing, 0
1 could mean on, true, positive, 1
2 could mean wheels on a bicycle, eyes on a face, hands, 2

Just some examples where I imagine it is because they are the most heavily used for certain features, it would almost impossible to change them (especially 1 as the return value of a perl module)

3) Declaring a variable, which you never (appear to) use

This is annoying if, like me, you use Class::Std to create inside out objects, as you need to declare a hash for your attributes, but this has is never referred to again in the code.

Now, whilst I understand that if you don't use a variable, don't declare it, in this case you are using it, just via the Class::Std methods of the accessors created. However, much like the declaration of numbers, it isn't looking at the context in which you use the declared variable. Again, in this case I have had to wrap these with a ## no critic {} ## use critic in order to not have it fail.

--

So as I said at the beginning, all I want to say is what a faff and the reason is as follows:

We are using Perl::Critic extensively here in New Pipeline Development, and it does force our hand to a Good Coding Practice, but I can't help but wonder here if a couple of these new standards are just a little too overzealous, and causing some things to be overcritised (i.e. a faff).

As with many things, it will take time to get used to programming in advance of critic'ing the code, but I think some of these new features need a little tweaking.