CODEWITHSUNDEEP
regexperl
chrisyyy
chrisyyy

Reputation: 45

Not moving files in perl grep in if expression

I have the current subroutine written to move files in my current directory to a /junk directory off my current directory if they don't have a certain file extension. For some reason, none of the files are getting moved, and I think it has to do with my unless expression. I've only been working in Perl for about a week so any help is very much appreciated!

sub clean_old_runs {

    my $current = cwd();
    opendir(DIR, $current);

    mkdir 'junk';

    my @files = readdir(DIR);

    my $num_of_files = scalar(@files);

    for(my $n = 0; $n < $num_of_files; $n++) {
        unless ( grep {/\.ext1$/} @_ or grep {/\.ext2$/} @_ or grep {/\.pl$/} @_) { 
            move("$current/@_", "$current/junk/@_");
        }
    }

    close DIR;
}

Upvotes: 0

Views: 83

Answers (1)

b4hand
b4hand

Reputation: 9770

There's actually more than one thing wrong with your code. Here's a complete working example:

#!/usr/bin/env perl

use Cwd;
use File::Copy;

sub clean_old_runs {
    my $current = cwd();
    opendir(DIR, $current);

    mkdir 'junk';

    my @files = readdir(DIR);

    my $num_of_files = scalar(@files);

    foreach my $file (@files) {
        unless ($file =~ /\.ext[12]$/ or $file =~ /\.pl$/) {
            move("$current/$file", "$current/junk/$file");
        }
    }

    close DIR;
}

clean_old_runs();

I'll walk through the wrong pieces of your original code one at a time:

my @files = readdir(DIR);

my $num_of_files = scalar(@files);

for(my $n = 0; $n < $num_of_files; $n++) {
      # ...
}

Here @files contains the actual file names. Your for loop doesn't actually iterate over those files. Thus, I've changed the loop to use a foreach iteration since you already have an array containing the files that you care about.

Next your unless logic is in fact wrong as you suspected:

unless ( grep {/\.ext1$/} @_ or grep {/\.ext2$/} @_ or grep {/\.pl$/} @_) { 
    move("$current/@_", "$current/junk/@_");
}

In particular, the @_ variable is the arguments to your function, which doesn't appear to be given any, but it's unclear from context since you don't show any invocation of clean_old_runs. Regardless, it appears you want to operate on @files inside this function. Furthermore, in my version, I'm already iterating over @files, so it doesn't make sense to use grep here, but in your version you would be running the grep many times for each file if you ran it in your loop.

I combined the two patterns to simplify things as /\.ext[12]$/ matches both /\.ext1/ and /\.ext2/.

Upvotes: 1

Related Questions