Should a subroutine always return explicitly?
If it perlcritic
says "no returns in sub is wrong", what is the alternative if you really don't need them?
I have developed two distinctly bad habits:
- I am explicitly assigning variables to the $ main :: namespace.
- Then I play with these variables in subs.
For example, I could do.
#!/usr/bin/perl
use strict;
use warnings;
@main::array = (1,4,2,6,1,8,5,5,2);
&sort_array;
&push_array;
&pop_array;
sub sort_array{
@main::array = sort @main::array;
for (@main::array){
print "$_\n";
}
}
sub push_array{
for ( 1 .. 9 ){
push @main::array, $_;
}
}
sub pop_array {
for ( 1 .. 3 ){
pop @main::array;
}
}
I don't do this all the time. But in the above sense, it makes sense because I can decouple the operations, no need to worry about passing values back and forth, and it usually looks clean to me.
But as I said, the perl critic is talking about his wrong - because there is no return ..
So, can anyone interpret what I am trying to do and suggest a better way to approach this coding style in perl? eg. Am I kind of doing OOP?
source to share
In short - yes, you basically do OO, but in a way that confuses everyone.
The danger of doing this kind of thing is that you are acting from a distance. This is bad coding style and must be looking elsewhere entirely for what might break your code.
This is usually why "globals" should be avoided whenever possible.
For a short script, it doesn't really matter.
Regarding return values - Perl returns the result of the last expression by default. (See .: return
)
(If there is no explicit return, the routine, eval, or FILE automatically returns the value of the last expression evaluated.)
The reason Perl crit means:
Require all routines to end explicitly with one of the following: return, carp, croak, die, exec, exit, goto, or throw.
Routines without explicit return statements at their ends can be confusing. It can be tricky to infer what will be returned.
Also, unless the programmer had in mind that there was a significant return value and omitted the return statement, some of the internal data of the subroutine might leak out.
Perlcritic is not always right, although if there is a compelling reason to do what you are doing, disable it. As long as you think about it and know about the risks.
Personally, I find it better to style to explicitly return something, even if it's simple return;
.
Anyway, by refactoring your code in (raw) OO form:
#!/usr/bin/perl
use strict;
use warnings;
package MyArray;
my $default_array = [ 1,4,2,6,1,8,5,5,2 ];
sub new {
my ( $class ) = @_;
my $self = {};
$self -> {myarray} = $default_array;
bless ( $self, $class );
return $self;
}
sub get_array {
my ( $self ) = @_;
return ( $self -> {myarray} );
}
sub sort_array{
my ( $self ) = @_;
@{ $self -> {myarray} } = sort ( @{ $self -> {myarray} } );
for ( @{ $self -> {myarray} } ) {
print $_,"\n";
}
return 1;
}
sub push_array{
my ( $self ) = @_;
for ( 1 .. 9 ){
push @{$self -> {myarray}}, $_;
}
return 1;
}
sub pop_array {
my ( $self ) = @_;
for ( 1 .. 3 ){
pop @{$self -> {myarray}};
}
return 1;
}
1;
And then call it with:
#!/usr/bin/perl
use strict;
use warnings;
use MyArray;
my $array = MyArray -> new();
print "Started:\n";
print join (",", @{ $array -> get_array()} ),"\n";
print "Reshuffling:\n";
$array -> sort_array();
$array -> push_array();
$array -> pop_array();
print "Finished:\n";
print join (",", @{ $array -> get_array()} ),"\n";
It can probably be tweaked a bit, but hopefully this illustrates - inside your object, you have an internal "array" which you then "make" with your calls.
The result is the same (I think I reproduced the logic, but I don't completely believe it!), But you're fine.
source to share
If the function doesn't mean to return anything, there is no need to use return
!
No, you are not using any aspect of OO (encapsulation, polymorphism, etc.). What you are doing is called procedural programming. There is nothing wrong. All my work in nuclear power plants was written in this style.
The problem is with the use @main::array
, and I'm not saying you can shorten this to @::array
. Qualified names come out of strict checks, so they are much more error prone. Mistyped var name won't come across as easily, and it's easy to compromise two pieces of code using the same variable name.
If you are only using one file, you can use my @array
, but I assume you are using @main::array
because you are accessing it from multiple files / modules. I suggest placing our @array
in a module and exporting it.
package MyData;
use Exporter qw( import );
our @EXPORT = qw( @array );
our @array;
1;
Any hint of a variable name (such as a prefix or suffix) to indicate that this is a variable used in many modules would be nice.
By the way, if you want to create an object it will look like
package MyArray;
sub new {
my $class = shift;
my $self = bless({}, $class);
$self->{array} = [ @_ ];
return $self;
}
sub get_elements {
my ($self) = @_;
return @{ $self->{array} };
}
sub sort {
my ($self) = @_;
@{ $self->{array} } = sort @{ $self->{array} };
}
sub push {
my $self = shift;
push @{ $self->{array} }, @_;
}
sub pop {
my ($self, $n) = @_;
return splice(@{ $self->{array} }, 0, $n//1);
}
my $array = MyArray->new(1,4,2,6,1,8,5,5,2);
$array->sort;
print("$_\n") for $array->get_elements();
$array->push_array(1..9);
$array->pop_array(3);
I have improved my interface a bit. (The sort shouldn't print. It would be nice to nudge different things and appear differently than the three items.)
source to share