I was reviewing some code I had written for a simple RPG dice algorithm (although there's already a good module for this,

Game::Dice) and I realized again that I have a prefererence for functions that can fit on one screen. One strategy is breaking up the code into smaller routines but I sometimes like to compact it vertically as much as possible first.

This function

`roll`

, given a string of "dice language," should return the results of such a dice roll. An example of this would be "3d10+1" to roll three 10-sided dice and then add 1, or "4d6b3" which says to roll four 6-sided dice and take the best three.

Here's the function before the refactor:

```
sub roll {
my $input = shift;
die unless $input =~ /d/;
if ( $input =~ /(\d*)d(\d+)\s*(\D?)\s*(\d*)/ ) {
my $num = $1 || 1;
my $die = $2;
my $plus = $3;
my $end = $4;
my $total = 0;
my @dice;
for my $count ( 1 .. $num ) {
my $single = int( rand($die) ) + 1;
push @dice, $single;
print "$single\n";
}
if ( $plus eq 'b' ) {
if ( $end > $num ) {
$end = $num;
}
@dice = sort { $b <=> $a } @dice;
$#dice = $end - 1;
}
for my $die (@dice) {
$total += $die;
}
if ( $plus eq '+' ) {
$total += $end;
}
elsif ( $plus eq '-' ) {
$total -= $end;
}
elsif ( $plus eq '*' ) {
$total *= $end;
}
elsif ( $plus eq '/' ) {
$total /= $end;
}
return $total;
}
return;
}
```

The first thing I did is to delete the first of this pair of lines, which was redundant, because the line that follows also checks the format of the input:

```
die unless $input =~ /d/;
if ( $input =~ /(\d*)d(\d+)\s*(\D?)\s*(\d*)/ ) {
```

But instead of having that big

`if`

block, I changed it to this:

return unless $input =~ /(\d*)d(\d+)\s*(\D?)\s*(\d*)/;

Then I combined these:

```
my $die = $2;
my $plus = $3;
my $end = $4;
```

into this:

my ($die,$plus,$end) = ($2,$3,$4);

Once I decided I didn't need to print each individual die as it was rolled, I could reduce this:

```
for my $count ( 1 .. $num ) {
my $single = int( rand($die) ) + 1;
push @dice, $single;
print "$single\n";
}
```

to this:

push @dice, int(rand($die))+1 for ( 1..$num );

Then, I changed this:

```
if ( $end > $num ) {
$end = $num;
}
```

To use the postfix

`if`

:

$end = $num if $end > $num;

and this:

```
for my $die (@dice) {
$total += $die;
}
```

to use postfix

`for`

:

$total += $_ for @dice;

One thing I like to do with an

`if/else`

chain like this:

```
if ( $plus eq '+' ) {
$total += $end;
}
elsif ( $plus eq '-' ) {
$total -= $end;
}
elsif ( $plus eq '*' ) {
$total *= $end;
}
elsif ( $plus eq '/' ) {
$total /= $end;
}
```

is to compress it like this:

if ( $plus eq '+' ) { $total += $end }
elsif ( $plus eq '-' ) { $total -= $end }
elsif ( $plus eq '*' ) { $total *= $end }
elsif ( $plus eq '/' ) { $total /= $end }

Since it's still short in width and the syntax can lined up to be quite readable.

So the final version of the refactored function is:

sub roll {
my $input = shift;
return unless $input =~ /(\d*)d(\d+)\s*(\D?)\s*(\d*)/;
my $num = $1 || 1;
my ($die,$plus,$end) = ($2,$3,$4);
my $total = 0;
my @dice;
push @dice, int(rand($die))+1 for ( 1..$num );
if ( $plus eq 'b' ) {
$end = $num if $end > $num;
@dice = sort { $b <=> $a } @dice;
$#dice = $end-1;
}
$total += $_ for @dice;
if ( $plus eq '+' ) { $total += $end }
elsif ( $plus eq '-' ) { $total -= $end }
elsif ( $plus eq '*' ) { $total *= $end }
elsif ( $plus eq '/' ) { $total /= $end }
return $total;
}

Now you can make things a

*lot* smaller (see

Perl Golf examples) but readability is important to me, and I think this is arguably as readable as the original. I was actually a little surprised that

perltidy barely touched the

`if/elsif`

structure, just screwing up the alignment a little on the first line:

```
if ( $plus eq '+' ) { $total += $end }
elsif ( $plus eq '-' ) { $total -= $end }
elsif ( $plus eq '*' ) { $total *= $end }
elsif ( $plus eq '/' ) { $total /= $end }
```

The code doesn't strictly adhere to

Perl Best Practices, which is something I like to use as a guide for the most part, but

perlcritic (which is based on Perl Best Practices) doesn't start to complain until the

`cruel`

setting, then bringing up things like postfix

`if`

, postfix

`for`

, and

`unless.`

How would you make it smaller while still maintaining readability?