HiveBrain v1.2.0
Get Started
← Back to all entries
snippetMinor

Merge Sort in Perl

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sortperlmerge

Problem

I'm new to Perl and have successfully written a merge sort program. I'll paste the entire program below, but would like mostly a review of the subroutines mergeSort() and merge().

I tried to use pass-by-reference as much as possible to make as it as fast as possible. Is there ways I can make this better/more optimized yet?

For example in my merge() subroutine, there is a line:

@$arr[$r] = $b[$j];


Where $arr is a pointer to an array, $b is an array, and $r and $j are both integer indexes. I am assigning the value at index $j in array $b to the $r index of my dereferenced array @$arr.

Is there a way to instead assign by dereferencing like in C where I might do arr = b[j] then arr++, or is this the best way? It seems like maybe I'm processing the entire array to change one element, but I probably just don't fully understand.

```
#!/usr/bin/perl -w
#Program usage: perl PROGRAM LIST_OF_INTEGERS
#example:
#perl mergeSort.pl 3 2 1 139 17 -3 3 5 0 1 9 -33 3 0 5335 4 -3 3

use strict;

### MAIN ###

# ensure a list has been passed with program call
if($#ARGV == -1) {
print STDERR "ERROR: You must specify a list of integers.\n".
"EXAMPLE: perl mergeSort.pl 1 7 20 11 2\n\n";
exit(-1);
}

# print outputs
print "\nBEFORE: [";
for $_ (@ARGV){
print $_, " ";
}
print "]\n";

### merge sort ###
print scalar (@ARGV), " elements\n"; # print # elements
my @sol = @ARGV; # copy list
mergeSort( \@sol );

# more printing
print " AFTER: [";
for $_ (@sol){
print $_, " ";
}
print "]\n";

# ensure sort was correct, comparing with built-in sort
print "CONFIRMED SORT WAS CORRECT? ";
my @sorted = sort {$a $b} @ARGV; # built in numerical sort

my $i = my $j = 0;
while( $i $b[$j]){
@$arr[$r] = $b[$j];
$j++;
}
else{
@$arr[$r] = $a[$i];
$i++;
}
$r++;
}

#once the while loop is broken, this means a or b has been emptied,
#so simply need t

Solution

Here are some thoughts on your code

-
Always prefer use warnings to -w on the command line or shebang line

-
People whose first language is Perl will thank you for using only lower case letters, decimal digits and underscores in local variable names. Capital letters are reserved for globals such as constants or package names

-
Reduce your comments as far as possible, and try to use identifiers that are self-explanatory. Comments break the flow of comprehension and should be reserved for truly confusing sections. Think of it like the friend sitting next to you on the sofa "helpfully" explaining the movie to you

-
Get used to the implicit context of expressions and reduce your use of scalar. For instance, check whether any command-line parameters have been supplied with unless ( @ARGV ) { ... }

-
Get used to the statement modifier forms of if, while, unless and for, which allow things like return if @$arr == 1

-
Get used to variable interpolation. By default arrays are interpolated into double-quoted strings with spaces separating the elements, which lets you write things like printf "\nBEFORE: [@ARGV]\n";. It also lets you compare (short) arrays with just "@sol" eq "@sorted"

-
Get used to using and and or for control flow instead of && and ||. They have better readability and their low priority will help you write what you mean without excessive parentheses

-
Get used to using list assignment, particularly when copying the parameters of a subroutine

-
Remember that $#arr is the last index of @arr, so scalar(@$arr)-1) is $#$arr

Here's how I would implement your algorithm. The only optimisation I can see immediately is that there's no need to copy the arrays in the merge subroutine. It is simple to access the data in-place using the dereference operator ->

#!/usr/bin/perl

use strict;
use warnings;

unless ( @ARGV ) {
    warn "ERROR: You must specify a list of integers\n",
         "EXAMPLE: perl merge_sort.pl 1 7 20 11 2\n\n";
    exit -1;
}

printf "\nBEFORE: [@ARGV]\n";
print scalar @ARGV, " elements\n\n";

my @sol = @ARGV;
merge_sort(\@sol);

printf "AFTER: [@sol]\n";
print scalar @sol, " elements\n\n";

print "Confirmed sort was correct? ";

my @sorted = sort {$a  $b} @ARGV;
print "@sol" eq "@sorted" ? "TRUE\n\n" : "FALSE\n\n";

sub merge_sort {

    my ($arr) = @_;

    return if @$arr == 1;

    my $bound = int( @$arr / 2 );
    my @left  = @$arr[ 0 .. $bound-1 ];
    my @right = @$arr[ $bound .. $#$arr ];

    merge_sort( \@left );
    merge_sort( \@right );

    merge( \@left, \@right, $arr );
}

sub merge {

    my ($a_ref, $b_ref, $arr) = @_;

    my ($i, $j, $r) = (0, 0, 0);

    while ( $i [$i] [$j] ) {
            $arr->[$r++] = $a_ref->[$i++];
        }
        else {
            $arr->[$r++] = $b_ref->[$j++];
        }
    }

    $arr->[$r++] = $a_ref->[$i++] while $i [$r++] = $b_ref->[$j++] while $j < @$b_ref;
}

Code Snippets

#!/usr/bin/perl

use strict;
use warnings;

unless ( @ARGV ) {
    warn "ERROR: You must specify a list of integers\n",
         "EXAMPLE: perl merge_sort.pl 1 7 20 11 2\n\n";
    exit -1;
}

printf "\nBEFORE: [@ARGV]\n";
print scalar @ARGV, " elements\n\n";

my @sol = @ARGV;
merge_sort(\@sol);

printf "AFTER: [@sol]\n";
print scalar @sol, " elements\n\n";

print "Confirmed sort was correct? ";

my @sorted = sort {$a <=> $b} @ARGV;
print "@sol" eq "@sorted" ? "TRUE\n\n" : "FALSE\n\n";



sub merge_sort {

    my ($arr) = @_;

    return if @$arr == 1;

    my $bound = int( @$arr / 2 );
    my @left  = @$arr[ 0 .. $bound-1 ];
    my @right = @$arr[ $bound .. $#$arr ];

    merge_sort( \@left );
    merge_sort( \@right );

    merge( \@left, \@right, $arr );
}


sub merge {

    my ($a_ref, $b_ref, $arr) = @_;

    my ($i, $j, $r) = (0, 0, 0);

    while ( $i < @$a_ref and $j < @$b_ref ) {

        if ( $a_ref->[$i] < $b_ref->[$j] ) {
            $arr->[$r++] = $a_ref->[$i++];
        }
        else {
            $arr->[$r++] = $b_ref->[$j++];
        }
    }

    $arr->[$r++] = $a_ref->[$i++] while $i < @$a_ref;
    $arr->[$r++] = $b_ref->[$j++] while $j < @$b_ref;
}

Context

StackExchange Code Review Q#91855, answer score: 4

Revisions (0)

No revisions yet.