more useful options PerlMonks

### comment on

 Need Help??
Hi orange,

The main problem you're having is in your is_prime subroutine.

You are calculating primes based on whether a number N is divisible by any value from 1 to N, when you only need to be looking at all odd values between 3 and sqrt(N), but including the special case of 2.  Additionally, you should be caching the values (you may have thought you were doing this with my @prime_;, but the correct way would be to use a hash.  (Additionally, though you were checking the value with if (\$prime_[1] == \$num) { \$b = 1; }, you were still letting the entire loop run its course, and thus defeating its purpose).

Update:  But see grinder's important comment, below, on an even further degree of optimization.

Update:  bart caught a bug in my program; namely, that it needs to test for division by 2!  I've added this in the code below.

Here's a rewrite of the entire program:

#!/usr/local/bin/perl use strict; use warnings; use Tk; my \$NumOfPrimes = 0; my (\$colr, \$counter); my(\$o, \$s) = (250, 20); my(\$pi, \$x, \$y) = (3.1415926, 0, 0); print "type the maximum number for the prime spiral \n"; my \$lastnumber = <>; print "please wait \n"; my \$mw = MainWindow->new; my \$c = \$mw->Canvas(-width => 500, -height => 500); \$c->pack; \$c->createLine(50, 250, 450, 250); \$c->createText(10, 250, -fill => 'blue', -text => 'X'); \$c->createLine(250, 50, 250, 450); \$c->createText(250, 10, -fill => 'blue', -text => 'Y'); my \$num = 0; my \$order = 0; my \$pnts = 0; my \$pntcycle = 0; OUTER: for (my \$i= 1; \$i <= 250; \$i += 1) { \$order=\$order+1; if (\$order == 5) {\$order = 1;} ++\$pntcycle; if (\$pntcycle == 3) { \$pntcycle = 1; ++\$pnts; } \$counter = 0; while (\$counter <= \$pnts) { \$counter++; ++\$num; # This is the more common way to increment a n +umber if (\$num == \$lastnumber + 1) { last OUTER } my \$b = is_prime(\$num); if (\$b != 0) {\$colr = "white"; \$NumOfPrimes = \$NumOfPrimes + + 1;} if (\$b == 0) {\$colr = "black";} if (\$order == 1) {\$x = \$x + 1;} elsif (\$order == 2) {\$y = \$y - 1;} elsif (\$order == 3) {\$x = \$x - 1; } elsif (\$order == 4) {\$y = \$y + 1; } \$c->createText( \$x+\$o, \$y+\$o, -fill => "\$colr", -text => ' +.'); } } MainLoop; BEGIN { # Cache primes for future use, but using a *hash*, NOT an array! # Start with the value "2", which lets us avoid my %primes = ( 2 => 1 ); sub is_prime { my (\$num) = @_; if (exists(\$primes{\$num})) { return \$primes{\$num}; } # Only need to calculate up to sqrt(\$num). # This is VERY important, especially with larger numbers. # If N is divisible by some factor \$f, less than sqrt(\$N), # then it's also divisible by some other factor \$k = \$N/\$f # which MUST be larger than sqrt(\$N), and vice versa. my \$sqrt = int(sqrt(\$num)); # Check for division by 2 (special-case) (\$num % 2) or return 0; # We've tried 2 as a factor, so any other factors must be odd # (and prime, for that matter). for (my \$j = 3; \$j <= \$sqrt; \$j += 2) { if (0 == \$num % \$j) { # It's NOT a prime. return 0; } } # Cache the prime number and return '1' return \$primes{\$num} = 1; } }

A couple of other points ...

1. You should use warnings, to get lots of valuable debugging information, like the fact that \$lastnumber was declared twice.
2. my (\$order,\$pntcycle,\$pnts,\$b,\$num,\$p) = (0); likely does NOT do what you want.  It's only setting the value of \$order to zero.  To set them all equal to zero, do something like my \$order = \$my \$pntcycle = my \$pnts = ... = my \$p = 0;, or individually (the way I did above).
3. It's not generally a good idea to make everything global, especially values into and out of a subroutine.  You really should be passing \$num to is_prime(), and returning the value which you assign to \$b.
4. You don't need to quote numbers when you put them into a hash -- \$prime_[\$p] = "\$j"; is better written \$prime_[\$p] = \$j.
5. It's more "Perl-like" to increment a variable with ++\$var.  \$var = \$var + 1 looks too much like, well, BASIC. :-)
6. Many will find your code easier to read if you indent it.

s''(q.S:\$/9=(T1';s;(..)(..);\$..=substr+crypt(\$1,\$2),2,3;eg;print\$..\$/

In reply to Re: ulam's spiral too slow by liverpole
in thread ulam's spiral too slow by orange

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":

• Are you posting in the right place? Check out Where do I post X? to know for sure.
• Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
• Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
• Want more info? How to link or or How to display code and escape characters are good places to start.

Create A New User
Domain Nodelet?
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others lurking in the Monastery: (3)
As of 2021-08-01 18:04 GMT
Sections?
Information?
Find Nodes?
Leftovers?
Voting Booth?
My primary motivation for participating at PerlMonks is: (Choices in context)

Results (14 votes). Check out past polls.

Notices?