tonyz has asked for the wisdom of the Perl Monks concerning the following question:

Dear Perl Monks,

The script I'm writing opens a data file I've already constructed, using it to create a hash. Next, it opens a directory of .gif files, reads the directory contents into an array, and closes the directory. Then, it uses the hash to search and replace on the array. When I print the array to STDOUT, I get a satisfactory list. The script creates a new directory. I want the new directory to contain new .gif files, renamed according to the search and replace. I tried to pump the contents of the array into the new directory. The script seems to execute, but I end up with a new, but empty directory.

open IMAGEHASH, "results.txt" or die "Couldn't open file:$!"; my %image_hash = ( ); my $search = undef; my $replace = undef; while (<IMAGEHASH>) { ($search, $replace) = ($_=~m/(.*) (.*)/); if ($search ne '' && $replace ne '') { $image_hash{$search} = $replace; } } opendir (GIFDIR, "/home/tony/perl/pmbencor") || die ("Cannot open dire +ctory."); my @the_files = grep { $_ ne '.' && $_ ne '..' } readdir(GIFDIR); closedir GIFDIR; foreach my $file (@the_files) { foreach $key (keys %image_hash) { $file=~ s/$key/$image_hash{$key}/g; } } mkdir ("newdir", 0777) || print $!; opendir (NEWDIR, "/home/tony/perl/newdir") || die ("Cannot open direct +ory."); use Cwd; my $here = cwd; my $there = '/home/tony/perl/newdir'; use File::Copy; foreach my $file (@the_files) {move ($here, $there)} closedir NEWDIR;

It seems to me there's a problem with the last six lines of code...I learned how to open, read and close directories from a search of the archives, but this has brought me up short...Thanks for your help!

Replies are listed 'Best First'.
Re: How do I move array contents into a directory?
by shmem (Chancellor) on Aug 29, 2008 at 16:09 UTC
    foreach my $file (@the_files) {move ($here, $there)}

    You should check for success. Always. Always!

    foreach my $file (@the_files) { move ($here, $there) or die "something weird happened moving '$here' to '$there': $ +!\n" }
Re: How do I move array contents into a directory?
by Zenshai (Sexton) on Aug 29, 2008 at 16:06 UTC
    Looks to me like you're not specifying which file to move.

    Here:
    foreach my $file (@the_files) {move ($here, $there)}
    I think you are essentially trying to move the cwd several times. Maybe this would work better:
    foreach my $file (@the_files) {move ($here . '/' . $file, $there . '/' + . $file)}
Re: How do I move array contents into a directory?
by kyle (Abbot) on Aug 29, 2008 at 16:29 UTC

    Starting after your %image_hash setup:

    my $gifdir_name = "/home/tony/perl/pmbencor"; opendir my $gifdir_dh, $gifdir_name or die "Can't opendir '$gifdir_name': $!"; my @orig_files = grep { !/^\.\.?$/ } readdir $gifdir_dh; closedir $gifdir_dh; my $there = '/home/tony/perl/newdir'; mkdir $there, 0777 or die "Can't mkdir $there: $!"; my $key_pattern = join '|', map { quotemeta } keys %image_hash; use File::Copy; # this should really be at the top somewhere foreach my $file (@orig_files) { my $new_name = $file; $new_name =~ s/($key_pattern)/$image_hash{$1}/g; move "$gifdir_name/$file", "$there/$new_name" or die "Can't move '$file' to '$new_name': $!"; }

    Notes:

    • You may want to grep out all files that start with a dot, not just "." and "..". In that case, just use !/^\./.
    • I don't think "$there" is a very good variable name (but I haven't changed it).
    • Your original code treated the %image_hash keys as patterns (that is, a key of "^" would have matched anything), but my code treats them as literal strings. If you want the pattern behavior, change the quotemeta map to "map { "(?:$_)" }".
    • Given "%image_hash = ( x => 'y', y => 'z' )", your code could have done two replacements on an "x", changing it to "z" (but not definitely). Mine will only do one replacement.
    • If you have "%image_hash = ( abc => 'x', abcd => 'y' )", and the $file is "abcd", I'm not sure which will match. If that's a concern, you may want to go back to a replacement loop and sort according to length or something.
    • Since your source and destination directories are on the same drive, you could use rename instead of move
    • I haven't tested what I wrote.
Re: How do I move array contents into a directory?
by toolic (Bishop) on Aug 29, 2008 at 16:06 UTC
    You loop over @the_files, but you tried to move a directory ($here) instead of the ($file). Try this:
    foreach my $file (@the_files) {move ($file, $there)}
Re: How do I move array contents into a directory?
by Illuminatus (Curate) on Aug 29, 2008 at 16:15 UTC
    Much of your script does nothing -- other than that, everything is great :)
    What are you trying to accomplish with:
    foreach my $file (@the_files) { foreach $key (keys %image_hash) { $file=~ s/$key/$image_hash{$key}/g; } }
    You modify a local variable without doing anything with it.
    Then, your actual attempt at moving the files does nothing, because you do not catenate a file name to your src and dst directories. You are also using 'move', but your initial explanation said 'new' files, implying you wanted copies.
    It looks like what you want is some modification of the above nested loop within the bottom move/copy loop. You need the file name from @the_files to append to $here, and the replaced name to append to $there.
    Finally, that nested substitution loop is dangerous. It may not properly process files where names are similar, such as 'game.gif' and 'agame.gif'. Best to use 'eq' to match the filename first, before doing the substitution.
      You modify a local variable without doing anything with it.

      He is not modifying a local variable, he is using an alias for the array elements. So it should have an effect.

Re: How do I move array contents into a directory?
by TGI (Parson) on Aug 29, 2008 at 17:25 UTC

    Your image hash parsing code makes the regex engine work harder than necessary. I've reworked this bit to show several alternatives.

    The problem is that /.* .*/ creates a lot of unnecessary backtracking. The regex engine is greedy, so it will scan the whole string and match it to the leading '.*'. Then it has to back track and remove one character at a time until it finds a space. If you write /[^ ]* .*/, then the engine can read until it finds a space, then move on the next match criterion, must be a space, match that and slurp in the rest of the string to match the final '.*'. I once read a great article that talked about this in detail, but I can't track it down.

    my %image_hash = ( ); while (<IMAGEHASH>) { # Limited scope of $search/$replace # You could just use split to split on whitespace: my ($search, $replace) = split; # Or if you want to split on the first single space character, and + force only 2 items: # my ($search, $replace) = split ' ', $_, 2; # or you could use a regex: # my ($search, $replace) = m/([^ ]*) (.*)/); # Suppress uninitialized warnings. # check for non-zero length, # although eq '' checks should be fine, too. { no warnings 'uninitialized'; if ( length $search and length $replace ){ warn "Found uplicate entry for: $search\n" if exists $image_hash{$searc}; $image_hash{$search} = $replace; } } }

    Warning: all code untested.


    TGI says moo

Re: How do I move array contents into a directory?
by FlatBallFlyer (Acolyte) on Aug 29, 2008 at 17:33 UTC
    I'm taking a different stab at your problem. I'm not 100% sure about your intent, but if I understand your problem, here is a solution. (I'm sure that there will be more elgant prose to accomplish)
    my @the_files = ('apple', 'pear', 'food'); my %image_hash = ('a'=>'z', 'o'=>'u', ); foreach my $file (@the_files) { my $newFile = $file; foreach my $key (keys %image_hash) { my $replaceVal = %image_hash->{$key}; $newFile =~ s/$key/$replaceVal/g; } print "move $file, $newFile or die 'Bummer!' \n"; }
    This performs all s// actions in the hash on each file name. The output from the above snippit is:
    move apple, zpple or die 'Bummer!' move pear, pezr or die 'Bummer!' move food, fuud or die 'Bummer!'

      If I use warnings and diagnostics, it also says,

      Using a hash as a reference is deprecated

      (D deprecated) You tried to use a hash as a reference, as in %foo->{"bar"} or %$ref->{"hello"}. Versions of perl <= 5.6.1 used to allow this syntax, but shouldn't have. It is now deprecated, and will be removed in a future version.

      Instead of "%image_hash->{$key}", say "$image_hash{$key}".