| Subject: | existing code is too slow, complex, and verbose |
Your code is very complex, in comparison to what is needed. Perl does
nearly everything you wanted very easily. Only the padding routine --
which is sort of weird, anyway -- is not trivial. I have attached a
patch that will replace much of your code with simpler Perl, or with the
use of List::Util, which is much faster, more robust, and has been core
since perl 5.7.3.
--
rjbs
| Subject: | simplify.patch |
diff -ru Array-PAT-1.0/Makefile.PL Array-PAT-rjbs/Makefile.PL
--- Array-PAT-1.0/Makefile.PL 2003-10-10 15:00:18.000000000 -0400
+++ Array-PAT-rjbs/Makefile.PL 2006-11-03 10:23:14.000000000 -0500
@@ -4,4 +4,7 @@
WriteMakefile(
'NAME' => 'Array::PAT',
'VERSION_FROM' => 'PAT.pm', # finds $VERSION
+ PREREQ_PM => {
+ 'List::Util' => 0, # minimum version unchecked
+ },
);
diff -ru Array-PAT-1.0/PAT.pm Array-PAT-rjbs/PAT.pm
--- Array-PAT-1.0/PAT.pm 2003-10-21 22:50:38.000000000 -0400
+++ Array-PAT-rjbs/PAT.pm 2006-11-03 10:21:28.000000000 -0500
@@ -14,53 +14,31 @@
@EXPORT = qw(in_array array_unique array_count_values array_merge array_pad shuffle); #put the subroutine names in here
use strict;
+use List::Util;
+
#checks to see if an item is in the specified array
sub in_array{
my ($check, @array) = @_;
- my $retval = 0;
- foreach my $test (@array){
- if($check eq $test){
- $retval = 1;
- }#if
- }#foreach
- return $retval;
+ for (@array) { return 1 if $_ eq $check }
+ return 0;
}#in_array
#remove all duplicate entries in an array
sub array_unique{
- my (@array) = @_;
- my @nodupes;
- foreach my $element (@array){
- if(in_array($element, @nodupes) == 0){
- @nodupes = (@nodupes, $element);
- }#if
- }#foreach
- return @nodupes;
+ my %seen = map { $_ => 1 } @_;
+ return keys %seen;
}#array_unique
#returns a hash using the values of the input array as keys and their frequency in input as values
sub array_count_values{
- my(@array) = @_;
- my @unduped = array_unique(@array);
- my %rethash;
- foreach my $mykey (@unduped){
- $rethash{$mykey} = 0;
- }#foreach
- foreach my $key (@array){
- my $value = $rethash{$key};
- $value++;
- $rethash{$key} = $value;
- }#foreach
- return %rethash;
+ my %seen;
+ for (@_) { $seen{ $_ }++ };
+ return %seen;
}#array_count_values
#merges the elements of two or more arrays together so that the values of one are appended to the end of the previous one
sub array_merge{
- my(@array1, @array2) = @_;
- foreach my $element (@array2){
- @array1 = (@array1, $element);
- }#foreach
- return @array1;
+ return @_;
}#array_merge
#Returns a copy of the input padded to size specified by pad_size with value pad_value.
@@ -70,52 +48,28 @@
sub array_pad{
my($pad_size, $pad_value, @array) = @_;
#this is just to get the absolute value of the pad_size
- my $temp = $pad_size;
- if($pad_size < 0){
- $temp = $temp * -1;
- }#if
+ my $abs_size = abs $pad_size;
#first let's see if the pad_size is less than or equal to the array size, return 0 no padding
- my $input_size = @array;
- if($temp <= $input_size){
+ my $diff = $abs_size - @array;
+ if($diff <= 0) {
return 0;
}#if
#since it didn't return 0, time to pad
#if the pad_size is < 0, pad on the left, else the right
+ my @padding = ($pad_value) x $diff;
if($pad_size < 0){
- my @retval;
- $temp = $temp - $input_size;
- for(my $i=0;$i<$temp;$i++){
- unshift(@array, $pad_value);
- }#for
- return @array;
+ unshift @array, @padding;
}#if
else{
- $temp = $pad_size - $input_size;
- for(my $i=0;$i<$temp;$i++){
- push(@array, $pad_value);
- }#for
- return @array;
+ push @array, @padding;
}#else
+ return @array;
}#array_pad
#returns an array in random order
sub shuffle{
- my(@array) = @_;
- srand;
- my $size = @array;
- my $random = int(rand($size));
- my @tracker;
- @tracker = (@tracker, $random);
- my @retval;
- @retval = (@retval, $array[$random]);
- while(@retval < @array){
- $random = int(rand($size));
- if(in_array($random, @tracker) == 0){
- @tracker = (@tracker, $random);
- @retval = (@retval, $array[$random]);
- }#if
- }#while
- return @retval;
+ my @shuffled = List::Util::shuffle @_;
+ return @shuffled
}#shuffle
1;