import-bot (20211) [Avatar] Offline
#1
[Originally posted by jmg092548]

I'm very new to Perl programming. Would appreciate it if anyone has
comments/criticisms/suggestions re: style, clarity, "idiomaticity", etc.,
re: the following exercise.

TIA,

Jim Guinness


#! /usr/bin/perl
# primes finder exercise, per Johnson
# find primes between 3 and $max

my $max;
my @primes = (2);

print "Enter the max value:
";
chomp($max = <STDIN>smilie;

# assume each candidate is prime by modulo'ing
# it with known primes; when one of the results
# is zero we know it's not prime

for my $candidate (3..$max) {
my $is_prime = 1;
for (@primes) {
$is_prime &&= $candidate % $_;
last unless $is_prime;
}
push @primes,$candidate if $is_prime;
}

print "Primes up to $max are: @primes
";
import-bot (20211) [Avatar] Offline
#2
Re: newbie -- RFC re: prime-finding exercise
[Originally posted by jmg092548]

Thanks Andrew! What a cool thing to be able to get your comments this way!!

> Well, it looks pretty good but I'd change that &&= line -- we know
> that $is_prime is always 1 entering the loop so we really don't need
> to do a boolean test on it, simply use an ordinary assignment depending
> on the modulo operation:
>
> $is_prime = 0 unless $candidate % $_;
>

Pretty obvious once you mention it! I think this was a case of "technique in
earnest search of a reason to use it", a newbie peril, for me at least.

Determined to use an assignment operator, I also considered initializing
$is_prime to 1 and then saying:

$is_prime *= $candidate % $_;

But I need to keep firmly in mind that, other things being equal, clarity is
more important than cleverness. smilie

More to the point for my understanding maybe, is to remember that if we're
looking for a False, there's no reason to remember what happened on other loop
iterations; as soon as we find one we're done.

I'm up to Ch. 6 so far and I really like your book and your approach. A great
learning aid in combination with O'Reilly's Llama, Camel, and Cookbook. I look
forward to the rest of the chapters.

Again, thanks.

Jim Guinness
import-bot (20211) [Avatar] Offline
#3
Re: newbie -- RFC re: prime-finding exercise
[Originally posted by jandrew]

> Pretty obvious once you mention it! I think this was a case of
> "technique in earnest search of a reason to use it", a newbie peril,
> for me at least.

Nothing wrong with experimenting with new techniques!

> But I need to keep firmly in mind that, other things being equal,
> clarity is more important than cleverness. smilie

Of course, cleverness is cool too -- wait until you see Abigail's
method of determining prime numbers with a regex in chapter 10 (just
don't try to calculate all the primes up to 100,000 with it smilie

andrew
import-bot (20211) [Avatar] Offline
#4
Re: newbie -- RFC re: prime-finding exercise
[Originally posted by jandrew]

Well, it looks pretty good but I'd change that &&= line -- we know
that $is_prime is always 1 entering the loop so we really don't need
to do a boolean test on it, simply use an ordinary assignment depending
on the modulo operation:

$is_prime = 0 unless $candidate % $_;

This seems more readable to my eyes and is marginally faster as well.
For a better speed increase note that we do not have to test all
current primes as divisors, merely the ones less than the square root
of $candidate (if there isn't a factor less than that, then there
certainly isn't a factor greater than that). So your inner loop
becomes:

for (@primes) {
$is_prime = 0 unless $candidate % $_;
last unless $is_prime;
last if $_ > sqrt($candidate);
}

This is a much bigger speed gain (try it both ways with the $max set at
10000). However, even this is about 3 times slower than the sieve
program in the book because it is doing extra work. But if you want
to calculate lots of primes (say 100,000 or more) even the sieve
becomes too expensive and using bit vectors is more memory efficient,
so we might want to use the Bit::Vector module which has prime
functionality built in:

#!/usr/bin/perl -w
use strict;
use Bit::Vector;

my $limit = $ARGV[0];
my $set = Bit::Vector->new($limit+1);
$set->Primes();
my @primes = $set->Index_List_Read();

foreach my $prime (@primes){
print "$prime is prime
";
}

This is around 3 times faster than my sieve when using 100,000 for
the limit. Of course, I realize this takes away the fun and
experience of coding your own. Your code is fine and quite readable
and Perlish (aside from the &&= thing I mentioned already). Nice
going.

regards,
andrew