http://qs321.pair.com?node_id=11117895


in reply to The 15 Puzzle

I'm proud of it, so give a nonexpert a break. That said, any constructive feedback would be most welcome.

For a non-expert it's pretty good. There are a few inconsistencies but I'm sure you can spot those and polish them out in due course. The only bit which really grates is this:

# GENERATE THE BOARD my @board; my $inversions; GEN: @board = (0, shuffle(1..15)); # @board = (1,0,2..15); # for testing $inversions = 0; for my $a(1..15) { for my $b(1..15) { ++$inversions if($a<$b and $board[$a]>$board[$b]) } } goto GEN if $inversions % 2;

There's really no need for a goto in here. You are clearly aware of conditional loops in Perl as you've used them elsewhere in this script. Let's re-write this to avoid the goto, avoid the special variables $a and $b and make it marginally more efficient by only checking the triangle rather than the square.

my @board; my $inversions = 1; while ($inversions % 2) { @board = (0, shuffle(1..15)); $inversions = 0; for my $x (1 .. 15) { for my $y ($x + 1 .. 15) { $inversions++ if $board[$x] > $board[$y]; } } }

Hopefully I have not altered the logic of your board construction at all, just tweaked the code to do the same thing but in a slightly more Perlish way. For completeness I would probably put this in its own subroutine and just call my @board = setup_board() in the main script as the setup is entirely independent of the rest of the script.

Replies are listed 'Best First'.
Re^2: The 15 Puzzle
by msh210 (Monk) on Jun 10, 2020 at 20:48 UTC

    Your

    my $inversions = 1; while ($inversions % 2) { @board = (0, shuffle(1..15)); $inversions = 0; for my $x (1 .. 15) { for my $y ($x + 1 .. 15) { $inversions++ if $board[$x] > $board[$y]; } } }

    bothers me a little, because the code is asserting we have one inversion, and then that we have none, and then counting them. It seems… wrong, somehow, to assert at the start that we have one inversion, beginning counting them — especially because that assertion is followed immediately by a contradictory one. So I switched it to

    my $inversions; do { $inversions = 0; @board = (0, shuffle(1..15)); # @board = (1,0,2..15); # for testing for my $x(1..15) { for my $y($x+1..15) { ++$inversions if $board[$x]>$board[$y] } } } while $inversions % 2;

    I'd appreciate your letting me know what you think.

    $_="msh210";$"=$\;@_=@{[split//,uc]}[2,0];$_="@_$\1";$\=$/;++$_[0]for$...1;print lc substr crypt($_,"@_"),1,6

      Both versions of the loop are effectively the same as far as the algorithm goes. By moving the conditional to the end you have avoided the need for the initial flag value. I just picked 1 as it makes $inversions % 2 a true value but it could equally have been -1 or -9999 or any other (odd) value if that were to make it seem less wrong. You could even go so far as to set up a constant called INITIAL_INVERSIONS and set that to have the arbitrary flag value - TIMTOWTDI.

      It's best to code in a way that's right for you. If the semantics of the variables are most important to you then that's clearly the way to go. There's no computational penalty for this and anything that makes the code easier to maintain, whether objectively or for the specific maintainer, has to be beneficial.

      Just the process of analysing this and re-working the loop will have been, I hope, a worthwhile exercise. Thanks for taking the time to do so.

Re^2: The 15 Puzzle
by msh210 (Monk) on Jun 10, 2020 at 09:55 UTC

    Thanks!

    $_="msh210";$"=$\;@_=@{[split//,uc]}[2,0];$_="@_$\1";$\=$/;++$_[0]for$...1;print lc substr crypt($_,"@_"),1,6