On 15/03/2015 00:25, Ilan Shlossman wrote:
Hi ,
I asked this question on linkedin but I think the best place to get
answers is here ...
I have a perl cod that check a range of numbers and print for each
number if it's prime number or not prime number.
So why its not working properly ? ...
#!C:\strawberry\perl\bin\perl
use strict;
use warnings;
&main;
sub main {
&Check_Prime_Number();
}
sub Check_Prime_Number {
my $Number;
my @Numbers = (3 .. 6);
for (@Numbers) {
&CheckIfprime($_);
}
sub CheckIfprime {
my $num = shift;
my $value = 1;
my $I;
my $remainder;
our @div_array;
for ($I = 2 ; $I < $num ; $I++) {
$remainder = $num % $I;
# print "remainder = $remainder\n";
if ($remainder == 0) {
print " - $num is NOT a prime number -\n";
#exit;
}
else {
push @div_array, $remainder;
}
}
if ($value !~~ @div_array) { # if value is not in the array
print " - $num is A prime number -\n";
}
}
}
---------------------------
the output:
- 3 is A prime number -
- 4 is NOT a prime number -
- 4 is A prime number -
- 5 is A prime number -
- 6 is NOT a prime number -
- 6 is NOT a prime number -
- 6 is A prime number -
Hi Ilan
I've reformatted your program so that it's a little easier to read. It
would make it easier for you to work on your own code if you indented it.
I'm not clear what your array @div_array is for, as you only ever test
that it contains the number 1 ($value never changes). What is happening
is that, even for the non-primes, the `for` loop comes to an end and "is
A prime number" is printed for everything. To fix it you need to add a
`return` statement where your commented `exit` is. That will exit the
subroutine as soon as a divisor is found.
Here's a few more points:
- You don't need a shebang line at the top of your program when you are
working on Windows. It doesn't do any harm, but perl will ignore it
except for some command-line switches
- Don't use an ampersand when calling subroutines. just `main()` is
correct. Whatever source taught you to do that is about twenty years out
of date
- You don't need so many separate subroutines. I think just one that
checks whether a number is prime is fine
- Don't declare one subroutine within another. It doesn't limit the
scope of the inner subroutine, and just makes your code slightly less
readable
- Variables should be declared as late as possible, preferably at the
point they are first used
- People familiar with perl will thank you for using only lower-case
letters in your local identifiers. Capitals are reserved for global
identifiers such as package names
- It is much clearer to iterate over a range than to use a C-style `for`
loop. The line
for ($I = 2 ; $I < $num ; $I++)
is better written
for my $i (2 .. $num-1)
- The smart match operator ~~ has been marked as experimental and its
use is no longer considered good practice
I've shown how I would write your algorithm below. I've dropped the use
of @div_array as I didn't understand its relevance, and the code works
fine without it. I hope that helps.
Rob
use strict;
use warnings;
my @numbers = (3 .. 6);
check_if_prime($_) for @numbers;
sub check_if_prime {
my ($num) = @_;
for my $i ( 2 .. $num-1 ) {
my $remainder = $num % $i;
if ( $remainder == 0 ) {
print " - $num is NOT a prime number -\n";
return;
}
}
print " - $num is A prime number -\n";
}
**output**
- 3 is A prime number -
- 4 is NOT a prime number -
- 5 is A prime number -
- 6 is NOT a prime number -
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/