in reply to Dice.pm
Here are some comments, take them as you will:
$_ = $parameter; $parameter =~ /^([0-9]+)d([0-9]+)/; $n_dice = $1; $n_sides = $2;
Why would you assign $parameter to $_, but then never take advantage of it? Also, the usual idiom for getting $1, $2, etc. is to grab the result of the pattern match in list context. And there's a shortcut for [0-9] in perl regular expressions--\d. Thus, this bit of code could be rewritten like so:
$_ = $parameter; ($n_dice,$nsides) = /^(\d+)d(\d+)/;
With all of the subsequent if statements matching against $_ instead of $parameter as well. Also, you make extra work with your loop variables:
for (my $I=1; $I <= $n_dice; $I++){ $rolls [$I-1] = int(rand($n_sides))+1;
Why start $I at 1 if you're going to subtract 1 everywhere you use it? Just use the usual idiom:
for (my $i = 0; $i < $n_dice; $i++) { $rolls[$i] = int(rand($n_sides))+1; ...
or perhaps better than using the C-style for loop, a more perly one:
for my $i (0..($n_dice-1)) { $rolls[$i] = int(rand($n_sides))+1; ...
Your subroutine appears to just return the total. I wonder if it would be more generally useful to return the rolls as well (possibly for some non-gaming activity). Here's one way to do that:
return wantarray ? @rolls : $total;
That returns the rolls themselves in list context, or just the total in scalar context.
Or if it's really just the totals you want, why restrict the subroutine to only one phrase? You could pass multiple phrases as a single space separated string or as a list and return a list of totals.
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: Re: Dice.pm
by talwyn (Monk) on Dec 01, 2003 at 14:52 UTC |