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/


Reply via email to