###########################################################
### not gonna complain about strict, for whatever reason
use MP3::Info;

die "Must use parameters and seperator\n (SEPERATOR ARTIST TITLE YEAR 
+ALBUM GENRE COMMENT)" unless @ARGV;
$sep=shift;
@tokens=@ARGV;
while(<*.mp3>) # while is wrong as you move files around, 
               # should be for (grep /\.mp3$/i, <*>)
               # to insure getting files like "bla.MP3" 
### indent your code!
{
  my $tag=get_mp3tag($_) or (push(@errors,"No tag for $_\n")   and nex
+t);
$nn='';

### now you're using <space><sep><space> as a separator
### I wanted <sep> only.  

for $i (@tokens){$nn.=$tag->{$i}." $sep ";}
my $z=rindex($nn,$sep);
my $len=length $sep;
substr($nn,$z,$len+1,'');
chop $nn;
$nn.='.mp3';

### the last 6 lines should've been something like:
### $nn = join($sep, map $tag->{$_}, @tokens) . ".mp3";
### and this is not my personal preferrence, but a more
### readable, straight to the point, honest to god statement

print "Renamed $_ to $nn \n" if rename $_,$nn; 
### where is my error check? 

}
print "\n\n";
print @errors;
[download]

Hope this helps!!

for $i (@tokens){$nn.=$tag->{$i}." $sep ";}
my $z=rindex($nn,$sep);
my $len=length $sep;
substr($nn,$z,$len+1,'');
chop $nn;
$nn.='.mp3';
[download]

use MP3::Info;
use strict; 
#
# what's boo_radley listening to?
# in CODE tag friendly format :)
#
my $fn = 'F:\Warlock Pinchers\Deadly Kung Fu Action '.
         '_ Pinch a Loaf\Warlock Pinchers - ' .
         'Deadly Kung Fu Action _ Pinch a Loaf '.
         '- 06 - Black in Black.mp3';

my $tag=get_mp3tag($fn) || die "unable to get tag.";

# tags to incorporate into new filename
my @tokens = qw (ARTIST TITLE ALBUM GENRE); 
my $sep ="-";
my $nn;

# from buu's script.
#
for my $i (@tokens){$nn.=$$tag{$i}." $sep "} 
my $z=rindex($nn,$sep);
my $len=length $sep;
substr($nn,$z,$len+1,'');
chop $nn;
$nn.='.mp3';

output the labors of our work
print "\n$nn\n";

#using the $tag object as a hash (in hash slice form!)
print join (" - ", @$tag{@tokens}),".mp3";
[download]

unless (-e $fn) {
    print "Renamed $_ to $nn \n" if rename $_,$nn
} else {
    # for example ...
    push(@errors,"Tried to rename $_ to $nn, file exists\n")
}
[download]

Nice code review. Hopefully BUU will learn from it.

But a minor pedantic point:

If this isn't clear, any time an object uses a hash to store data, you can (usually, unless the module author was clever) access that hash by typecasting the instantiation of that object into a hash.

I'm guessing that the above was just poor choice of words but just in case a newbie reads it I wanted to clear up what appears to be a bit of confusion.

What you really mean is that any time an objects underlying data type is a hash then it can be manipulated just like a reference to a normal hash, ( there is no casting involved).  In fact due to the fact that object types in perl are orthogonal from the their data type (despite what ref() says) this statement can be generalized to say that an object reference can always be treated exactly the same as a reference to the underlying data type.  The clever bit only means that the author has used an underlying datatype (I'm guessing you meant CODE refs) that prevents introspection.

Anyway, as I said good review.

Yves / DeMerphq
---
Writing a good benchmark isnt as easy as it might look.