There is more to address here than algorithm efficiency. You need to learn more of
perls' features. One glaring example is the lack of return values from subroutines.
In perl subroutines can also be functons. Another example is the global variables
defined at the beginning of the program. Globals are variables that are used
anywhere in the program. They are usually frowned on. Let's take a look at one
global: @fire_items. It is used exclusively in the subroutine:
handle_fire().
handle_fire() is called each pass through the while block. The reason
for making @fire_items global is because we don't want to lose the
items between passes. The problem is if we want to use @fire_items in
the future we need to take into account that at least one subroutine is changing it
in an inobvious way. The use of many global variables makes it difficult to
optimize and debug a program. There are too many dependencies.
When designing a subroutine, pretend a manager or teacher has assigned you the task.
She has given the inputs, the outputs, and a few details. The rest of the program is
not written yet or is not shown to you. Changing a variable in the program is not an
option. Everything you need must be passed in and everthing you change must be
passed back out. You can break the rules as you become a better programmer or as the
situation dictates. While your learning, try to find a way to stick to them.
Here is the handle_fire() subroutine:
sub handle_fire {
my ($new, $x, $y) = @_;
my $skip = 0;
if($new){ push(@fire_items, $x.' '.$y); }
my $i = 0;
while($i < @fire_items) {
my ($xc, $yc) = split(' ',$fire_items[$i]);
splice(@fire_items,$i,1) and $skip = 1 unless $xc < $app->widt
+h + $images{'beam1'}->width;
if($skip){ $skip = 0; next; }
&erase_image($images{'beam1'}->width, $images{'beam1'}->height
+,$xc-10,$yc);
&put_image($images{'beam1'}, $xc, $yc);
$fire_items[$i]=join(' ',$xc + 10, $yc);
$i++;
}
}
Three items initially caught my eye:
- The use of an index to step through an array.
- The use of the & prefix to subroutines. Re-read perlsub. & doesn't always mean what you think.
- The use of @fire_items, %images, and $app which have not been passed to the subroutine.
This is how handle_fire() is called:
#draw gunshots, possibly add new one
handle_fire( $fire, $x, $y );
Let's change handle_fire( $fire, $x, $y ) to accept and return a reference to
@fire_items.
#draw gunshots, possibly add new one
$fire_items = handle_fire( $fire, $x, $y, $fire_items );
Let's keep $fire_items defined outside the while block to
maintain it's value between passes. Remember $fire_items is a
reference to @fire_items.
my $fire_items;
.
.
.
while (1) {
.
.
.
#draw gunshots, possibly add new one
$fire_items = handle_fire( $fire, $x, $y, $fire_items );
.
.
.
}
Here's our sub again (with some whitespace added).
sub handle_fire {
my ( $new, $x, $y ) = @_;
my $skip = 0;
if ( $new ){
push(@fire_items, $x.' '.$y);
}
my $i = 0;
while( $i < @fire_items ) {
my ( $xc, $yc ) = split(' ', $fire_items[$i] );
splice( @fire_items, $i, 1 ) and $skip = 1 unless $xc < $app->
+width + $images{beam1}->width;
if( $skip ){
$skip = 0;
next;
}
erase_image( $images{beam1}->width, $images{beam1}->height, $x
+c-10, $yc );
put_image( $images{beam1}, $xc, $yc );
$fire_items[$i] = join(' ', $xc + 10, $yc );
$i++;
}
}
In order to use $fire_items reference we'll need to de-reference it. We'll
also need to add a return and change the line that receives the passed variables.
sub handle_fire {
my ( $new, $x, $y, $fire_items ) = @_;
my $skip = 0;
if ( $new ){
push(@$fire_items, $x.' '.$y);
}
my $i = 0;
while( $i < @$fire_items ) {
my ( $xc, $yc ) = split(' ', $$fire_items[$i] );
splice( @$fire_items, $i, 1 ) and $skip = 1 unless $xc < $app-
+>width + $images{beam1}->width;
if( $skip ){
$skip = 0;
next;
}
erase_image( $images{beam1}->width, $images{beam1}->height, $x
+c-10, $yc );
put_image( $images{beam1}, $xc, $yc );
$$fire_items[$i] = join(' ', $xc + 10, $yc );
$i++;
}
return $fire_items;
}
Now let's look at the data structure. By using an Array of Arrays, we can eliminate
the joining and splitting of spaces from $x and $y.
if ( $new ){
push(@$fire_items, $x.' '.$y);
}
Becomes:
if ( $new ){
push @$fire_items, [$x, $y];
}
Or just:
push @$fire_items, [$x, $y] if $new;
and:
my ( $xc, $yc ) = split(' ', $$fire_items[$i] );
Becomes:
my ( $xc, $yc ) = @{ $fire_items[$i] };
and:
$$fire_items[$i] = join(' ', $xc + 10, $yc );
Becomes:
$$fire_items[$i] = [$xc + 10, $yc];
The splice is removing something from the array and going to the next item.
Since $fire_items is a reference, changing it changes an external variable.
Something we normally want to avoid if possible. Also $skip is only used to
get to the next index. First, we'll get rid of $skip. As far as I can tell
splice always returns a value:
unless ( $xc < $app->width + $images{beam1}->width ) {
splice( @$fire_items, $i, 1 );
next;
}
Now we'll add a temporary array to hold the values we're keeping and to avoid changing
the $fire_items array. This is what I ended up with:
sub handle_fire {
my ( $new, $x, $y, $fire_items ) = @_;
my $i = 0;
my @temp;
while ( $i < @$fire_items ) {
my ( $xc, $yc ) = @{ $fire_items[$i] };
next unless $xc < $app->width + $images{beam1}->width;
erase_image( $images{beam1}->width, $images{beam1}->height, $x
+c-10, $yc );
put_image( $images{beam1}, $xc, $yc );
push @temp, [$xc + 10, $yc];
$i++;
}
push @temp, [$x, $y] if $new;
return \@temp;
}
Since we are no longer using splice, we no longer need to track the index:
sub handle_fire {
my ( $new, $x, $y, $fire_items ) = @_;
my @temp;
foreach my $item ( @$fire_items ) {
my ( $xc, $yc ) = @$item;
next unless $xc < $app->width + $images{beam1}->width;
erase_image( $images{beam1}->width, $images{beam1}->height, $x
+c-10, $yc );
put_image( $images{beam1}, $xc, $yc );
push @temp, [$xc + 10, $yc];
}
push @temp, [$x, $y] if $new;
return \@temp;
}
For speed you could forgo not changing $fire_items, but pushing is
probably faster than splicing.
HTH,
Charles K. Clarkson
Clarkson Energy Homes, Inc.
| 