Thanks Shlomi for your Valuable comments.
On Mon, Sep 21, 2015 at 3:41 PM, Shlomi Fish <shlo...@shlomifish.org> wrote:
> 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/
>
>
>
--
*********************************************************
Don't ask them WHY they hurt you,
because all they'll tell you is lies and excuses.
Just know they were wrong, and try to move on.
**********************************************************
