Using warnings is a good start, but the rest of the strictures mantra is to use strict. Personally I like a little more white space around the place and have tuned Perl::Tidy to do it the way I like it.

You should avoid overloading variable names. You use both $int and @int. In some cases that can lead to very subtle and hard to find bugs. In fact for this code you don't need the array at all, just plonk the range straight into the for loop list.

Making those few adjustments gives:

#!/usr/bin/perl
use strict;
use warnings;

print "Type number you want factorised: \n";
chomp (my $no = <STDIN>);

print "The factors of $no are: \n";

foreach my $int (1 .. $no / 2) {
    my $div       = $no / $int;
    my $remainder = $no % $int;

    if ($remainder == 0) {
        print "$div\n";
    }
}
[download]

#!/usr/bin/perl

use strict;
use warnings;
use Data::Dumper::Concise;
use Math::Factor::XS qw(factors);

my $number = shift @ARGV;
print Dumper(my @factors = factors($number));
[download]

maebuf:

You've already received some good comments. I'll just make a couple here:

• In your loop, you're doing a divide and a modulus every iteration. You could do the divide only when the modulus leaves no remainder.
• You don't really need to store the result of the modulus in a variable since you're using it only in your if statement. So you could just move the calculation to your if statement.
• Similarly, you're creating an array from a sequence, and then using the sequence only in one place. You could put the sequence in the for loop where you use it.

Putting those together would give you:

foreach $int (1 .. ($no/2)) {
   if ($no%int == 0) {
      $div = $no / $int;
      print "$div\n";
   }
}
[download]

Algorithmically, you're doing quite a few operations that will *never* give you a factor, because your sequence includes even numbers other than two. You could generate a list that includes 2 and all the odd numbers up through half of the input number like this:

foreach $int (2, map {$_*2+1} (1 .. ($no/4)) {
  ....
[download]

I hope you find it helpful.

...roboticus

Update: If you want to crunch the code down, you could even combine most of it to a single statement:

print "$_\n" for grep { 0==$no%$_ } (2, map { 2*$_+1 } 1 .. $no/2);
[download]

Note: Another thing to watch for is that you're not restricting yourself to prime factors. I don't know if it matters to you, though.

Whether or not this is a toy program, you should also look at Math::Big::Factors: eg, factors_wheel($no, 1). Here's a hint in using the output of that function: increment hash values for each repeated factor returned, and you'll get a complete list of factors and their multiplicities.

You could collapse most of that into print "$_\n" for grep { 0 == $no % $_ } @int;, if you wanted to. But I have my doubts whether this is reliable for large numbers, because you'll be comparing zero to a tiny floating-point number, which is usually a bad idea.

What tiny floating-point numbers? '%' will always yield an integer, and since @int doesn't contain numbers exceeding $no, even $no/$_ for @int does not involve floats smaller than 1.

Oh, you're right. I was thinking of the technique of naively dividing a number by a factor, without my paying attention to the code. Thanks!

• use strict
• declare variables using 'my'.
• use 3 or 4 spaces per indent level, they accumulate fast once you get into longer programs.
• At about this level, you might start breaking your program into routines ...

#!/usr/bin/perl
use warnings;

my ( $number )  = read_user_number();
my ( $factors ) = factor( $number );
output_number( $factors );
[download]

Yes, I've added a complication ... though we generate an array of factors, I return and pass a reference to the array, rather than the list of values.

Cheers for all the pointers guys, really helpful! Since posting I've been carrying on with the book and learned about strict and my and after you all pointing them out they'll be in common use now! Plus, I can see how I've repeated alot of the calculations when they're not needed. Haven't got as far as grep yet but I can see the gist from this, and modules are pretty far in my future I think.

To summarise, B- for first code, A+ for personal development!

Thanks, M