[sf-perl] randomize particular lines

Quinn Weaver qw at sf.pm.org
Mon Mar 6 16:50:08 PST 2006


On Mon, Mar 06, 2006 at 04:05:47PM -0800, Bart Alberti wrote:
> Here is the idea I am working on.
> Bart Alberti

Randomize word order, huh?  I guess this is just a learning-Perl
exercise?  Anyway, I see many problems.  Please see my ##comments
preceded by double pound signs.

A few general observations:

- You obviously have an incomplete grasp of the language.  I would
suggest reading _Learning Perl_.  At the very least, you need to
read some basic documentation so you can, e.g., understand how rand
works, before you attempt to use it.  (For rand, see the perlfunc
man page; for $_, see the perlvar man page.)

- This is an example of obfuscated Perl code, which gives readers
headaches and gives Perl (and Perl programmers) a bad name.  For
instance, the line

   @scramble = sort { (-1,1)[rand 2] } @scramble

seems deliberately obscure.  I hope you don't write this kind of thing
at work!

#!/usr/bin/perl

#use warnings;
use strict;
my $dictionary = "textfile";
my ($word);
   open (DICT, $dictionary) or die "Cannot open $dictionary: $!";
   my @words = <DICT>; ## Bad variable name.  This gets an array of lines,
                       ## not an array of words.  I can see by your
                       ## following code that you know this, but you
                       ## should use another name to make things easier
                       ## on the reader.
   foreach (@words)
   {
    chomp($_);
 #   print "$_,\n";  #for debug reason
   } ## Your foreach block ends, meaning that 
   if ($_ % 3 == 1){   ## $_ is the line itself, not the line number.
                       ## You want to find every third line, right?
   $word = $words[rand @words]; ## rand returns fractional numbers.
                                ## Unfortunately Perl uses floating-point
                                ## numbers by default, so it allows
                                ## fractional array subscripts.  However,
                                ## you can't reliably take the 1.23'th element
                                ## of an array.  This expression will
                                ## sometimes get you undef, which is
                                ## probably not what you want.
    my @scramble = split(//, $word);
   @scramble = sort { (-1,1)[rand 2] } @scramble; ## This code cries out for
                                                  ## documentation.  It
                                                  ## also suffers from the
                                                  ## rand problem described
                                                  ## in my preceding comment.
  $word = $scramble; #if this line omitted, prints the whole file unchanged
  }
    foreach (@words) {print "$_\n"}; ## Here you are printing $_,
                                     ## which is unmodified (except
                                     ## for the chomp you did).
                                     ## You have been operating on $word,
                                     ## which is a copy of an element
                                     ## of @words.  So all your work is lost.
__END__


PS:  As of at least Perl 5.6, you can use open to create a lexically
scoped filehandle object, rather than a hard-to-handle package-scoped
filehandle typeglob.  So instead of

open (DICT, $dictionary) or die "Cannot open $dictionary: $!";

You could do

    open my $dict_fh, $dictionary or die "Cannot open $dictionary: $!";

That way you don't have to worry about whether you already used the
name DICT, and you can simply pass $dict_fh to other functions.  In
short, it scales better.  The other use of open isn't incorrect,
just suboptimal.

--
qw (Quinn Weaver); #President, San Francisco Perl Mongers
=for information, visit http://sf.pm.org/weblog =cut


More information about the SanFrancisco-pm mailing list