The problem is that as you remove elements you invalidate the index that you are using. However, Perl has a much better way of doing what you want. Consider:

use strict;
use warnings;

my @arry = (9,0,0,5,3,0,0,0,2,0,1,0);
print "@arry \n";
@arry = grep {$_} @arry;
print "@arry \n";
[download]

Prints:

9 0 0 5 3 0 0 0 2 0 1 0 
9 5 3 2 1
[download]

The magic is grep.

use strict;
use warnings;

my @arr = (9, 0, 0, 5, 3, 0, 0, 0, 2, 0, 1, 0);
print qq{@arr\n};

for my $idx ( reverse 0 .. $#arr )
{
    splice @arr, $idx, 1 unless $arr[$idx];
}
print qq{@arr\n};
[download]

This produces

9 0 0 5 3 0 0 0 2 0 1 0
9 5 3 2 1
[download]

I hope this is useful.

Cheers,

JohnGG

BTW: This is exactly the same kind of problem a for loop will run into if you change the array it is running over. So remember the old rule to never change an array you're iterating over unless you know exactly what you are doing.

Your algo is not that far of, just put the $i++ in an else block. (Or use a more perlish way, see the othe reply.)

while ( $i <= $#arry)
 {
 if ($arry[$i] == 0)
   {splice @arry, $i,1;}
 else
   {$i++;}
}
[download]

Or, alternatively, move backwards.

When you splice like this, items with a larger index shift forward, but those with a lower index remain in place — so the problem simply vanishes by looping back to front.

$i = @arry;
while ( --$i >= 0 )
 {
 if ($arry[$i] == 0)
   {splice @arry, $i,1;}
}
[download]

• Before the splice:

  9,0,0,5,3,0,0,0,2,0,1,0
    ^ $i is 1
  [download]

• After the splice:

  9,0,5,3,0,0,0,2,0,1,0
    ^ $i is 1
  [download]

  $arry[$i] is still 0

You need to keep spliceing while it remains zero, change your if to a while.

Now think about what happens when you've just spliced that last 0. $i points past the end of the array, so you need to add a check for that into the while condition:

while ( $i <= $#arry &&  $arry[$i] == 0 )
[download]

Having said all of that, GrandFather is right. Just use grep.

The problem you encountered with splice() is very common: Splicing elements in or out of a list upsets the numbering of the remaining elements. Often the solution is to traverse the list in reverse order, beginning at the high end. Thus, splice can be used much in the way you originally had it:

my $i = $#arry;
while ( $i >= 0 )
 {
   if ($arry[$i] == 0)
     {splice @arry, $i,1;}
   $i-- ;
}
[download]

$arry[ $_] or splice @arry, $_, 1 for reverse 0 .. $#arry;
[download]

Praveen, ikegami, Grandfather, etc all had wonderful ideas.

I'd use grep, but my code would probably look like this:

@array = (1, 0, 2, 0, 3, -1, 0, 4, 0, -5, 0, 123);
@nonzeros = grep {$_} @array;
[download]

or

@array = (1, 0, 2, 0, 3, -1, 0, 4, 0, -5, 0, 123);
@nonzeros = grep {$_ != 0} @array;
[download]

Both are based on the assumption that the array's contents are strictly numeric; if you want to remove strings, your grep would require a different regex.

my @arry = (9,0,0,5,3,0,0,0,2,0,1,0);
print "@arry \n";
@new_ary = grep{/[^0]/} @arry;
print "@new_ary \n";
[download]