On Feb 26, 2004, at 3:54 PM, Cy Kurtz wrote:

Hi all.

Hello.


[snip problem description]

The code(try not to laugh out loud):

I'll inline some comments.


#!/usr/bin/perl
#numpick.pl

#use strict;

This is easily fixed, see below.


#use warnings;

This problem takes a little more work to fix, but I'll explain it.


Below, when you generate a number and then loop through the list to check for duplicates, you always loop through the size the list will eventually be, not currently is. So the first number gets randomly generated, then you check 52 slots of @numbers that don't exist yet for a match. Then you randomly generate a second number, and check it against the first and 51 slots that don't yet exist. Rinse, repeat.

Every time you check a previously undefined slot, Perl issues a warning and since your program is doing a whole lot of that, that's a lot of warnings.

You do that again with @supers, later.

You would need to rework the logic in those places, stop checking the undef values, to silence the warnings. That's good though because it would also save a LOT of busy work.

Let me show you a common Perl idiom:

for my $value (@array_of_values) {
        # use value here...
}

This loops through all the values in the array, putting each in turn in $value. You can do something with $value and then move on to the next. If there are three values in the array, it will loop three times. For 10,000 values, it will loop 10,000 times. That's what you need, in those places.

#a program to pick lottery numbers
#select 50 numbers in 10 5 number tickets
#select 10 supernumbers
#all numbers range from 1 to 52
#select each number only once.

my $try=1;
my $numbers;
my $supers;

Here's you strict problem. Change both of the above $'s to @, since you are declaring arrays, not scalars and the code will compile with strict.


my $i=1;
my $j=1;
my $k=1;
my $l=1;
my $m=1;
my $n=1;

Ug, that's a lot of little counters. You could reuse most and cut the list a bit, but let's look at a better way...


#select 50 numbers
while($i<51){

Another way to write this loop is:


for my $i (1..50) {
        # use $i here, which will have 1, 2, 3...50
}

That eliminates the need for all though variables, since they are scoped to their loop. You can reuse $i in a later loop, as long as it's outside the original.

I'm going to stop going line by line there, as that covers most of this issues with your script. You can clean it up a lot and simplify it for sure, possibly by removing those bottom loops, which don't seem to do anything, and more.

I strongly encourage you to try to clean up your code a bit, as it would be a valuable lesson. However, I'll show you another way you might try this, just to spark some fresh ideas:

#!/usr/bin/perl

use strict;
use warnings;

# load two useful functions from a handy list manipulation module
# 'perldoc List::Util' for more info
use List::Util qw(first shuffle);       # standard in 5.8+

# get fifty unique random numbers
my @numbers = (1..52);                  # load array with 52 numbers
@numbers = shuffle @numbers;    # randomize array
splice @numbers, 0, 2;                  # drop the first two

# get ten powerballs, different method
my @powerballs;
until (@powerballs == 10) {             # loop until we get 10
        my $pick = int(rand 51) + 1;    # make a random number
        # ...and add number if it wasn't already in the list
        push @powerballs, $pick unless first { $_ == $pick } @powerballs;
}

# do output
print "\n";
while (@numbers) {
        printf "%02d  %02d  %02d  %02d  %02d  -  %02d\n",
                   splice(@numbers, 0, 5), shift @powerballs;
}
print "\n";

__END__

Hope that helps.

James


-- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] <http://learn.perl.org/> <http://learn.perl.org/first-response>




Reply via email to