note liverpole Hi [orange], <p>The main problem you're having is in your <c>is_prime</c> subroutine. </p><p>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.&nbsp;&nbsp;Additionally, you should be caching the values (you may have thought you were doing this with <c>my @prime_;</c>, but the correct way would be to use a hash.&nbsp;&nbsp;(Additionally, though you were checking the value with <c>if (\$prime_ == \$num) { \$b = 1; }</c>, you were still letting the entire loop run its course, and thus defeating its purpose). </p><p><b>Update</b>:&nbsp;&nbsp;But see [grinder]'s important comment, below, on an even further degree of optimization. </p><p><b>Update</b>:&nbsp;&nbsp;[bart] caught a bug in my program; namely, that it needs to test for division by 2!&nbsp;&nbsp;I've added this in the code below. </p><p>Here's a rewrite of the entire program: <c> #!/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 number 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; } } </c> </p><p>A couple of other points ... <ol> <li>You should use [doc://warnings], to get lots of valuable debugging information, like the fact that <c>\$lastnumber</c> was declared twice. <li><c>my (\$order,\$pntcycle,\$pnts,\$b,\$num,\$p) = (0);</c> likely does NOT do what you want.&nbsp;&nbsp;It's only setting the value of <c>\$order</c> to zero.&nbsp;&nbsp;To set them all equal to zero, do something like <c>my \$order = \$my \$pntcycle = my \$pnts = ... = my \$p = 0;</c>, or individually (the way I did above). <li>It's not generally a good idea to make <i>everything</i> global, especially values into and out of a subroutine.&nbsp;&nbsp;You really should be passing <c>\$num</c> to <c>is_prime()</c>, and returning the value which you assign to <c>\$b</c>. <li>You don't need to quote numbers when you put them into a hash -- <c>\$prime_[\$p] = "\$j";</c> is better written <c>\$prime_[\$p] = \$j</c>. <li>It's more "Perl-like" to increment a variable with <c>++\$var</c>.&nbsp;&nbsp;<c>\$var = \$var + 1</c> looks too much like, well, [wp://BASIC]. :-) <li>Many will find your code easier to read if you indent it. </ol> <div class="pmsig"><div class="pmsig-465654"> <hr /> <font size="1"> s''(q.S:\$/9=(T1';s;(..)(..);\$..=substr+crypt(\$1,\$2),2,3;eg;print\$..\$/ </font> </div></div> 610218 610218