Hi Uday,

please reply to the list.

I'm going to comment on your code.

On Mon, 21 Sep 2015 15:06:49 +0530
Uday Vernekar <vernekaru...@gmail.com> wrote:

> Hi all,
> 
> I wrote a simple Perl code to find the factorial of a number.
> Need feed back on the same.please correct me if i have done anything wrong
> 
> #!/usr/bin/perl
> use warnings;
> use strict;

It's good that you used string and warnings, but you should have some empty
lines between paragraphs.

> my $fact=1;

You should avoid pre-declaring your variables.

> print("Enter Number:");

This may not get output in time. You need to flush the output - see perldoc
IO::Handle .

> my $num=<STDIN>;

I personally prefer using <ARGV> or <> for short consistently than <STDIN>
which is more limited.

> if($num==0)
> {
>  print("factorial of $num=$fact\n");
> }

No need for this special case, and you have some duplicate code.

Furthermore , you should have spaces between the expressions, like:

«
if ($num == 0)
{
    print ("The factorial of $num is $fact\n");
}
»

> elsif($num<0)
> {
> print("please enter only positive integers \n");
> }

Your code is missing indentation and furthermore, this note should be outputted
to STDERR or use die or warn.

> else
> {
>    for (1..$num)
>     {
>      $fact*=$_;
>     }

It's a good idea not to use $_ as an implicit looping construct because it can
be overrided very easily. Use a lexical variable:

    for my $i (1 .. $num)
    {
        $fact *= $i
    }

It will also be a good idea to extract the code calculating the factorial into
its own subroutine, which will accept a number and return the result.

Finally note that Math::BigInt and Math::GMP already provide ->bfac() for
calculating the factorial, so you should use them instead of your own
homebrewed routines.

Regards,

        Shlomi Fish

-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
http://is.gd/i5eMQd - Emma Watson’s Interview for a Software Dev Job

If a tree falls down in the middle of the forest, and there's nobody in its
vicinity, it will still make a sound, because Chuck Norris can hear it.
    — http://www.shlomifish.org/humour/bits/facts/Chuck-Norris/

Please reply to list if it's a mailing list post - http://shlom.in/reply .

--
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