5
\$\begingroup\$

This is a project I always meant to do, but now that I've done it, I am wondering if/how I can do it better. I've made some fixes and done general testing on the parsing to make sure it works, but I sense there is more to do that could be very instructive to me in terms of organizing my code and thoughts.

This came about because I played several Sokoban games and wrote solutions only to notice that many levels were rotations of others, so I wanted to save time. There are Sokoban solvers out there, but I wanted to provide explanations with the solutions.

The code below takes a text file (it need only be one line - the comments provide one) and user-suggested rotations (argument 1 on the command line) and changes all directions(2U, 3L, 4D, 5R) to how the user wants the board rotated.

Some of it seems a bit brute force, though, especially the for loop and the regular expression (I've only recently moved beyond ending with just /g or /gi). In particular, I am wondering whether I should use an array instead, especially since I may want an option to convert the full words "up"/"right"/"left"/"down" to their rotational equivalents.

Any suggestions big or small are appreciated, whether it's technical or regarding code organization.

This seems like the sort of exercise that might help me learn other languages I'm not familiar with, so I wanted to see how things went with a language I was okay at, first.

Thanks to all who are able to contribute, and I hope my example is somewhat interesting.

################################
#sokoban solution rotator
################################
#sok.txt has lines like so: Your way to the end: 2U 4R 3D 5L 2u 4r 3d 5l.
#the command line argument is any combination of l r h u d v (d and v are equivalent to u, vertical flips. L/R rotate left/right, h = horizontal flip)
#

use strict;
use warnings;

my $file = "sok.txt";
my $dir;
my $anyDif; # checks for differences once we've parsed rotations

# this hash transforms a move to a different direction. By default each move points to itself.
my %rotate = ( 'u' => 'u',
  'r' => 'r',
  'd' => 'd',
  'l' => 'l');

if (!defined($ARGV[0]))
{
  die ("You need a command line argument (no spaces) to use rotations: R, L, H and V/U/D rotate right, left, horizontally and vertically.");
}

my @adjust = split(//, lc($ARGV[0]));

for $dir (@adjust)
{
  if ($dir eq 'r') # rotate right
  {
    ($rotate{'r'}, $rotate{'d'}, $rotate{'l'}, $rotate{'u'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  if ($dir eq 'l') # rotate left
  {
    ($rotate{'l'}, $rotate{'u'}, $rotate{'r'}, $rotate{'d'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  if (($dir eq 'v') || ($dir eq 'u') || ($dir eq 'd')) # vertical flip
  {
    ($rotate{'d'}, $rotate{'r'}, $rotate{'u'}, $rotate{'l'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  if ($dir eq 'h') # horizontal flip
  {
    ($rotate{'u'}, $rotate{'l'}, $rotate{'d'}, $rotate{'r'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  die ("I didn't recognize $dir. RLHVUD (case insensitive) are the only directions I do.");
}

for $dir (sort keys %rotate)
{
  #print "$dir becomes $rotate{$dir}\n";
  $anyDif += ($dir ne $rotate{$dir});
}

#print "Rotation: URDL => $rotate{'u'}$rotate{'r'}$rotate{'d'}$rotate{'l'}\n";

if (!$anyDif) { die("The rotations you requested don't change the puzzle's orientation."); }

open(A, "sok.txt") || die ("No sok.txt.");

while (my $line = <A>)
{
  print "Orig: $line";
  print "NEW:: ";
  $line =~ s/\b([0-9\*]*)([URDL])\b/$1 . newdir($2)/gei; # I included * with numbers because it means "push to the end"
  chomp($line);
  print "$line\n"; #this is to avoid orig/new appearing on the same line at the end of a file.
}
close(A);

##################################subroutine

sub newdir
{
  if ($_[0] eq lc($_[0]))
  {
    return $rotate{$_[0]};
  }
  return uc($rotate{lc($_[0])});
}
\$\endgroup\$
3
  • \$\begingroup\$ Looks interesting! I have not played the game, could you try explain what the codes in sok.txt represent? You say it represents how the user wants the board rotated, but why would he write 5L 3d for example? Why not just 1L 1D? If he flips the board 3 times down, wouldn't that be the same as 1 time down? since flipping 2 times down would return the board to the original configuration \$\endgroup\$ Commented Apr 27, 2017 at 19:20
  • 1
    \$\begingroup\$ @HåkonHægland with the rotations, yeah, I took care of some unlikely test cases. I suppose I could put in a check for too many letters in the command. As for the codes, they are just shorthand, e.g. move your player 3 squares left, then 2 up, 4 down, 1 right, then so forth. I suppose I could've used up/down/left/right as well. I don't know how much Sokoban you've played, but in youtube.com/watch?v=i_WmVBwEE4U the first part is 1U 3L 3U 1L 1U 2L 1D 2L 3D and now *R would send the box right because I'm too lazy to count. The 1's are optional of course. \$\endgroup\$ Commented Apr 27, 2017 at 21:41
  • \$\begingroup\$ Ok I think I understand, but there is still something in your code I don't get. Line 37: why does the moveU becomes R when rotating with L? If I rotate the board 90 degrees counter clockwise (i.e rotate left), the move U should become L, and not R? What am I missing here? \$\endgroup\$ Commented Apr 27, 2017 at 22:02

2 Answers 2

5
\$\begingroup\$

Please check for additional comments in the code,

use strict;
use warnings;

# no need to die() when open()
use autodie;

# this hash transforms a move to a different direction. By default each move points to itself.
my %rotate = (
  'u' => 'u',
  'r' => 'r',
  'd' => 'd',
  'l' => 'l'
);

if (!defined($ARGV[0]))
{
  die ("You need a command line argument (no spaces) to use rotations: R, L, H and V/U/D rotate right, left, horizontally and vertically.");
}

my @adjust = split(//, lc($ARGV[0]));
#/(fix stackexchange code coloring)

for my $dir (@adjust) {
  my @keys;

  # rotate right
  if ($dir eq 'r') { @keys = qw(r d l u); }
  # rotate left
  elsif ($dir eq 'l') { @keys = qw(l u r d); }
  # vertical flip
  elsif ($dir eq 'v' or $dir eq 'u' or $dir eq 'd') { @keys = qw(d r u l); }
  # horizontal flip
  elsif ($dir eq 'h')  { @keys = qw(u l d r); }
  else {
    die ("I didn't recognize $dir. RLHVUD (case insensitive) are the only directions I do.");
  }

  # check "hash slice"  
  @rotate{@keys} = @rotate{qw(u r d l)};
}

my $anyDif; # checks for differences once we've parsed rotations
for my $dir (keys %rotate) {
  #print "$dir becomes $rotate{$dir}\n";
  $anyDif += ($dir ne $rotate{$dir});
}

#print "Rotation: URDL => $rotate{'u'}$rotate{'r'}$rotate{'d'}$rotate{'l'}\n";

if (!$anyDif) { die("The rotations you requested don't change the puzzle's orientation."); }

# check "three argument open"
open(my $A, "<", "sok.txt");

while (my $line = <$A>) {
  print "Orig: $line";
  print "NEW:: ";
  $line =~ s/\b([0-9\*]*)([URDL])\b/$1 . newdir($2)/gei; # I included * with numbers because it means "push to the end"
  chomp($line);
  print "$line\n"; #this is to avoid orig/new appearing on the same line at the end of a file.
}
close($A);

##################################subroutine

sub newdir {
  my ($v) = @_;
  my $lc = lc($v);

  # check "ternary operator"
  return ($v eq $lc) ? $rotate{$v} : uc($rotate{$lc});
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ There is a lot of stuff in your code I'd seen but remember thinking, well, the shorthand is for advanced PERL programmers...and I never got around to really using it. I really appreciate this, because so much here is not just being economical for code golf sake. I usually wait a day or so before giving a best answer, but if there's a better one I'll really be thrilled. Thanks! \$\endgroup\$ Commented Apr 27, 2017 at 21:36
2
\$\begingroup\$

In addition to the points made in the previous answer, here are some other suggestions.

Layout

Two-space indentation makes the code hard to understand. You can use perltidy to automatically format your code using 4 spaces for each indentation level to make the code easier to read.

It is considered cleaner to omit parentheses for built-in function calls, as you do with sort and keys in this line:

for $dir (sort keys %rotate)

For cleaner code and consistency, you should do the same with defined, close, die, etc.

Semicolons are usually omitted after code block closing braces:

};

The code is cleaner as:

}

Documentation

It is good that you have comments at the top of the code to summarize its purpose. It is recommended to use plain old documentation (POD) which gives users manpage-like help with perldoc.

Things to document:

  • Input file name
  • Input file description
  • Required command-line description with a few specific examples

UX

When I run the code without any command-line arguments, the code shows me an error message and dies, as designed:

You need a command line argument (no spaces) to use rotations: R, L, H and V/U/D rotate right, left, horizontally and vertically. at ... line 25.

While it is great that you notify the user of an error, I don't understand the message. That is a lot of information packed into a long line of text. It might be better to direct the user to the full documentation to see specific examples.

It is purely a matter of preference, but I like to add a newline character (\n) to the end of die calls to suppress the program name and line number:

die("No sok.txt.\n")

Comments

Remove commented-out code to reduce clutter:

#print "$dir becomes $rotate{$dir}\n";

#print "Rotation: URDL => $rotate{'u'}$rotate{'r'}$rotate{'d'}$rotate{'l'}\n";

Warnings

My preference is to use a very strict version of warnings:

use warnings FATAL => 'all';

In my experience, the warnings have always pointed to a bug in my code. The issue is that, in some common usage scenarios, it is too easy to miss the warning messages unless you are looking for them. They can be hard to spot even if your code generates a small amount of output, not to mention anything that scrolls off the screen. This option will kill your program dead so that there is no way to miss the warnings.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.