At least your intuition is accurate, wrt the inefficiency of the code. Not that it's all that bad, and for a script like this, who cares if it runs in 3 seconds or 6?
Here's some suggestions:
- Look at all the places you use sort. Most, if not all, are unnecessary.
- Your reading and parsing of the file could be much more concise, with a payoff in efficiency. To wit:
while (<FILE>) { # line at a time
my( $game,$month,$day,$year,$one,$two,$three,$four,$five,$bonus) = s
+plit;
for ( $one,$two,$three,$four,$five ) {
$hash{$_}++;
}
$bonus{$bonus}++; # bonus
}
- Your inner "pushing" loops could be replaced with:
push @draw, ($key) x $hash{$key};
Let perl do the grunt work.
I might even combine the two "pushing" loops into something like the following-
foreach my $key ( keys %hash )
{
push @draw, ($key) x $hash{$key};
push @mega, ($key) x $hash{$key}
if exists $bonus{$key};
}
since, I'm assuming, a number can only appear in the 'bonus' column if it also appears at least once in one of the other columns.
- Your duplicate-detecting loop has a bug. You want to pick a new number if the current number is the same as any in your @picked array. I assume. Right?
If so, then you could use grep to do set membership tests on the array.
Even better would be to use a hash. You're doing many repeated set membership tests on it, and a hash is much better at this than an array. If you care about the order in which numbers are picked (and I assume you do), you could use Tie::IxHash to get an order-preserving hash.
Or in a case like this you could just use an auxiliary "seen" hash.
Here's a solution using grep:
my $number;
do {
$number = draw_regular();
} while ( grep { $_ == $number } @picked );
push @picked, $number;
Here's a solution using an aux hash:
my $number;
do {
$number = draw_regular();
} while ( exists $seen{$number} );
$seen{$number}++;
push @picked, $number;
