Uri's post is excellent as always, and I hope he doesn't mind me
piggybacking on it:
2009/10/13 Uri Guttman <u...@stemsystems.com>:
>>>>>> "JF" == Jesus Fernandez <jfer...@tigers.lsu.edu> writes:
>
> JF> Here is the script:
>
> it helps if your question and code are in the same posting. this will be
> a general code review as well as a solution to this last bug.
>
> JF> jaff
> JF> #!jesusafernandez/bin/perl
> JF> use warnings;
>
> use strict ;
>
> always use both strict and warnings
There is *no* good reason not to. It is not hard to use strict, and
the earlier you start doing it, the easier it will be to switch. Not
only that, but it will be easier to get help if you use strict,
because people will appreciate that you are doing as much as you can
to find bugs yourself before coming here.
> JF> while ($k>=2){
>
> you are just counting $k from 25 to 2, so show that in a for
> loop. either use the c style loop and count down or a reversed list:
>
> for my $k ( reverse 2 .. 25 ) {
>
> JF> if ($k==1) {last;}
>
> not needed if the loop is coded with for.
It's not needed for the while loop either. In a loop with condition
while ($k>=2), the loop body won't run when $k == 1.
You should trust the loop condition! Don't write an extra check "just in case".
> JF> $mean=(4*$n)/$k*($k-1);
>
> learn about white space. this isn't mathematics, people need to read the
> code. also learn about indenting. it would have helped you in solving this.
>
>
> my $mean = ( 4 * $n ) / $k * ( $k - 1 ) ;
>
> and if you do know precedence (same for basic math and perl) then the
> first set of parens isn't needed.
Also, the way you have written it is confusing. It is equivalent to:
my $mean = 4 * $n * ($k-1) / $k;
but it looks like you may have meant the very different:
my $mean = 4 * $n / ($k * ($k-1));
Either of these is more readable than your original, and both look
like the programmer meant what they wrote and didn't forget any
parens.
> JF> print "$coalt[$time]\n";
>
> that print is outside the time loop. if you had used strict you would
> have seen that this $time wasn't declared and found the bug. this
> statements needs to be inside the previous loop. also indenting would
> have helped you see what code is inside which block.
> JF> }
Actually, $time is declared because you are using the $time variable
for two different purposes. First, outside the for($time) loop you
say:
$time=(-log(rand)*$mean);
Then you say:
for ($time=0; $time<=24; $time++){
The first usage of $time is as a random variable with floating-point
value. The second is as an index with integer value. These are two
different conceptual objects, and you should give them different
names. Don't reuse variable names for different purposes.
If this was an accidental reuse, it would have show up under use
strict because you would have said:
my $time=(-log(rand)*$mean);
and then
for (my $time=0; $time<=24; $time++){
and the second declaration of "my $time" would have flagged a warning.
If it was a deliberate reuse, then it was a bad judgement and you have
now learned not to do it.
Phil
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
