Would you mind critiquing my script?

[marlowvoltron]

The code works, I just know it can be improved... I just want to make sure I'm using "die" right, I've had issues before this version where the .tar files I am trying to move would disappear if something went wrong. And fyi- I can't use modules due to permissions when installing

#!/usr/bin/perl -w

use strict;
use warnings;


my @files = glob "*.*";
my @tar_files = glob '*.tar.gz'; # All the tar files in the curr dir.
my @jpg_files = glob '*.jpg'; # All the .jpg files in the curr dir.

unless (@files) {
    die "No files here. Exiting!\n";
}

for my $tar_file (@tar_files) {
    my ($path, $row) = $tar_file =~ m/^LC8(\d{3})(\d{3})\d+LGN\d\d\.ta
+r.gz$/xms;
    print "Found $tar_file, with path $path, and row $row\n";
    my $dir = "p${path}_r${row}";
    print "...so... dir is /$dir\n";
    #if (! -e $dir) {
        #print "$dir dir not found...";
     
    #}
    {
    system ("mv $tar_file /data2/jennb/landsat8/antarctica/$dir") == 0
+ or die "can't move file!";
    print "moved $tar_file to /$dir\n";
    }
    {
    chdir "/data2/jennb/landsat8/antarctica/$dir" or die "can't cd to 
+/$dir !\n";
    print "...moving to /$dir...\n";
    system ("pwd");
    }
    {
    print "current contents: ";
    system ("ls");
    print "...untaring... please wait...\n";
    system ("tar -xzvf $tar_file --wildcards '*B8*'") == 0 or die "unp
+acking of $tar_file failed!\n";
    print "printing new contents...\n";
    system ("ls");
    print "...moving on...\n";
    }
    {
    chdir "/data2/jennb/landsat8/downloads";
    print "...listing remaining contents in /downloads...\n";
    system ("ls");
    }
}

print "tar files done...now jpgs...\n";

for my $jpg_file (@jpg_files) {
    my ($path, $row) = $jpg_file =~ m/^LC8(\d{3})(\d{3})\d+LGN\d\d\.jp
+g$/xms;
    print "Found $jpg_file, with path $path, and row $row\n";
    my $dir = "p${path}_r${row}";
    print "...so... dir is /$dir\n";
    
    system ("mv $jpg_file /data2/jennb/landsat8/antarctica/$dir") == 0
+ or die "Failed to move $jpg_file\n";
    print "Moved jpg to /$dir\n";
    
}
print "process complete!\n";

[dwm042]

1. You could improve readability by assigning those large directory strings to lexical variables.

my $target_dir   = "/data2/jennb/landsat8/antarctica/$dir";
my $download_dir = "/data2/jennb/landsat8/downloads";

2. I agree with Laurent_R. Look at the POSIX module and try using those commands instead of system("this") and system("that") everywhere. chdir() is a perl built-in, iirc.

3. Consider seeing if your functionality repeats enough to convert some of your code into subroutine/function calls. The for loops with tars and jpegs looks like the first place to start. Long stretches of linear code is a sign that you're not really trying to break the task down by function.

David

[karlgoethebier]

If you insist on using UNIX commands only you should consider using qx instead of system and wrap the calls into eval blocks and examine $?>>8 and $@ afterwards.

But you should keep in mind that Perl provides some other mighty weapons, like File::Find::Rule, IO::All, Cwd, File::Copy, Archive::Tar, Try::Tiny and IPC::Run. Some bro(s) mentioned this already ;-)

Edit: Fixed typo.

Best regards, Karl

[Laurent_R]

Hmmm, since you ask, using all these system calls to launch Unix shell commands is usually not considered to be the best practice, especially when there is an internal Perl command doing more or less the same, not to speak of numerous modules to work on files specifications, directory lists, etc. That being said, I am really not an extremist on these things, sometimes the Unix command gives a better control.

[Anonymous Monk]

Is there a compelling reason why perl must be used, and not a shell script or Makefile?

[Anonymous Monk]

Is there a compelling reason why perl must be used, and not a shell script or Makefile?

Since when is one required? Makefile syntax is horrible, and shell script not much better