1: #
   2: # Copies MP3s by genre tag using sources directory tree.
   3: # Script must be executed from the source's root
   4: # i.e. $srcdir must be a path on the same drive as where the script is executed from
   5: #
   6: # Criticism/advice appreciated
   7: # 
   8: 
   9: use strict;
  10: use MP3::Info;
  11: use File::Find;
  12: use File::Copy;
  13: use File::Basename;
  14: use File::Path;
  15: 
  16: my $genre = 'Country';
  17: my $srcdir = '/MP3 Test/';
  18: my $dstdir = 'c:/';
  19: my ($tag, $newdir, $path, $file, $dir);
  20: 
  21: find(\&wanted, $srcdir);
  22: 
  23: sub wanted {
  24:     open(FL, $File::Find::name);
  25: 
  26: if ((/\.mp3/)){
  27: 	$tag = get_mp3tag($_) or print "Could not access tag on $_\n" and next;
  28: 
  29: 	if ($tag->{"GENRE"} eq $genre) {
  30: 		$path = $File::Find::name;
  31: 		$dir = dirname($path);
  32: 		$file = basename($path);
  33: 		$newdir = $dstdir . $dir;
  34: 
  35: 		if (! -d $newdir ) { 
  36: 			mkpath ($newdir, 0666) || die "can't mkdir $newdir: $!\n";
  37: 		}
  38: 		copy($path, $newdir . '/' . $file) or die "copy failed: $!\n"; 
  39: 	}
  40: }
  41: 	close(FL);
  42: }

Replies are listed 'Best First'.
Re: Copy Mp3s By Genre
by Jaap (Curate) on Dec 14, 2003 at 13:13 UTC
    Just litle comment:
    If you declare all your variables at the top of your program you lose one advantage of 'use strict;'.

    If you declare your variables as late as possible, you cannot accidentally use the same name twice unintentionally under strict.
[reply]
Re: Copy Mp3s By Genre
by sacked (Hermit) on Apr 26, 2004 at 22:29 UTC
    Nice work! This looks good, and useful, too. Here are a few suggestions:

    You open a filehandle FL to each file you examine, but you never read from it. I would remove the code related to this filehandle:
    open(FL, $File::Find::name);
...
close(FL);

    [download]

    Inside a subroutine, you should use return rather than next. If you use the code as is, you will see warnings from Perl such as "exiting subroutine via next at script.pl line...":

    $tag = get_mp3tag($_) or print "Could not access tag on $_\n" and next;
    becomes
    $tag = get_mp3tag($_) or print "Could not access tag on $_\n" and return;


    For portability, use File::Spec to generate file or directory names:

    use File::Spec::Functions qw(catdir);
...
$newdir = catdir( $dstdir => $dir );

    [download]


    File::Path::mkpath raises an exception on error. You'll want to wrap that call in eval. Also, the second argument is a boolean flag that controls printing of the directories as they are created. The third argument is the mode, for which you'll need to have execute permissions on if you want to create subdirectories: 
    eval { mkpath($newdir, 0, 0755) };
if ($@) {
  print "can’t mkdir $newdir: $@";
}

    [download]


    Finally, as a simple style change, you can reduce the indentation level of the code in your wanted subroutine if you use the statement modifier forms of if/unless.


    Here is a different version of your script with the changes described above: 
    use strict;
use MP3::Info;
use File::Find;
use File::Copy;
use File::Basename;
use File::Path;
use File::Spec::Functions qw(catdir);

my $genre = 'Country';
my $srcdir = '/MP3 Test/';
my $dstdir = 'c:/';

find(\&wanted, $srcdir);

# note: now vars ($dir, etc.) scoped to subroutine (per Jaap)

sub wanted {
    # anchor regex to end of filename
    return unless /\.mp3\z/;

    my $tag = get_mp3tag($_) or print "Could not access tag on $_\n" a
+nd return;
 
    return unless $tag->{"GENRE"} eq $genre;

    my $path  = $File::Find::name;
    my $dir   = dirname($path);
    my $newdir= catdir( $dstdir => $dir );

    if (! -d $newdir ) {
        eval { mkpath($newdir, 0, 0755) };
        if ($@) {
            print "can’t mkdir $newdir: $@";
        }
    }

    # don't need basename($path), just pass copy()
    # a directory as the second argument
    copy($path => $newdir) or die "copy failed: $!\n"; 
}

    [download]


    You can make the script even more useful by allowing the user to specify the genre, source directory, and destination directories on the command line.


    --sacked
[reply]
[d/l]
[select]

Back to Craft