Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl-Sensitive Sunglasses
 
PerlMonks  

Comment on

( #3333=superdoc: print w/ replies, xml ) Need Help??
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);

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 );


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: $@"; }


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"; }


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

In reply to Re: Copy Mp3s By Genre by sacked
in thread Copy Mp3s By Genre by mumbles

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • Outside of code tags, you may need to use entities for some characters:
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.
  • Log In?
    Username:
    Password:

    What's my password?
    Create A New User
    Chatterbox?
    and the web crawler heard nothing...

    How do I use this? | Other CB clients
    Other Users?
    Others browsing the Monastery: (6)
    As of 2014-07-26 18:51 GMT
    Sections?
    Information?
    Find Nodes?
    Leftovers?
      Voting Booth?

      My favorite superfluous repetitious redundant duplicative phrase is:









      Results (178 votes), past polls