Newbie: Looking for comments on this (working) script

[Trebor A. Rude]

I've been trying to learn Perl in recent days, and was hoping the group
could offer some pointers on my first attempt to do something useful (to
me, anyway) with it. To that end, any constructive comments, suggestions,
etc. on this script would be appreciated.

The script takes a list of .ogg files on the command line and reads the tag
information from them (which come out in "key=value" form), then uses it to
construct a command to put the same tag information into a
corresponding .mp3 file (I prefer .oggs for listening on my computer, but
my car's CD player only works with audio or MP3 CDs). Here it is:

#!/usr/bin/perl

use warnings;
use strict;
use vars '$file';

my %options = (title => '-t',
artist => '-a',
album => '-A',
tracknumber => '-T');

sub run_id3v2 (@)
{
my @command = ("id3v2", @_, $file);
return system(@command) == 0;
}

foreach (@ARGV)
{
$file = $_;
my @output = `vorbiscomment -l \Q$file\E 2>&1`;

if ($? != 0)
{
print "Problems reading tags from $file, skipping it:\n", @output;
next;
}

my %comments = map { chomp; split /=/, $_, 2 } @output;

my @arguments = ();

# Also considered using (if this is strange indentation, thank
# cperl-mode):
#
# push @arguments,
# map {($options{$_}, $comments{$_})}
# grep exists $options{$_}, keys %comments;

push @arguments,
map {exists $options{$_} ? ($options{$_}, $comments{$_}) : ()}
keys %comments;

$file =~ s/\.ogg/.mp3/;

run_id3v2('-D') or die "Problem removing old tags from $file.";
run_id3v2(@arguments) or die "Problem adding new tags to $file.";
run_id3v2('-s') or die "Problem removing new id3v1 tags from $file.";
}

 

[Paul Lalli]

what's this supposed to do? Which perldoc can I find out in?

Do you even *try* before posting here? Gee, it's having to do with a
subroutine, you think *maybe* it might be in the documentation for
subroutines?

perldoc perlsub
it's called a prototype, although this particular one is rather useless.
Can you read the documentation and tell us why, Robin?

Paul Lalli

 

[Paul Lalli]

because he really isn't using any references? I scanned the perlsub docs,
and this is probably wrong, but I have to write a paper.
-Robin

Almost correct, actually. A prototype of (@) tells perl "Only accept a
list of arguments" (as opposed to something like ($\@), which says "accept
a scalar value followed by an actual named array". But accepting a list
of arguments is the default behavior for Perl subroutines. Therefore,
using a prototype of (@) is no different than not using a prototype at
all. (it is very different, however, from using an empty prototype, like
sub myfunc() )

To clarify, prototypes do not have to have anything to do with references.
They can be used to restrict the number and type of arguments passed to
subroutines. For example:

sub myfunc($$) {

This function will only accept two scalar values. They can be any scalar
values you want, be them strings literals, numeric literals, named scalar
variables, or yes, references. But the following function calls are all
permitted:

myfunc("foo", 'bar');
myfunc(1, $string);
myfunc('', 0);

none of which do any conversions to references. In this case, the
prototype could be in place to cause fatal errors for any of these
function calls:

myfunc ("foo", "bar", "baz");
myfunc (42);
myfunc ();
@ten = (1..10);
myfunc (@ten);


Paul Lalli

 

[Robin]

sub run_id3v2 (@)

what's this supposed to do? Which perldoc can I find out in?

Thanks,
-Robin

 

[John W. Krahn]

I've been trying to learn Perl in recent days, and was hoping the group
could offer some pointers on my first attempt to do something useful (to
me, anyway) with it. To that end, any constructive comments, suggestions,
etc. on this script would be appreciated.

The script takes a list of .ogg files on the command line and reads the tag
information from them (which come out in "key=value" form), then uses it to
construct a command to put the same tag information into a
corresponding .mp3 file (I prefer .oggs for listening on my computer, but
my car's CD player only works with audio or MP3 CDs). Here it is:

#!/usr/bin/perl

use warnings;
use strict;
use vars '$file';

You don't really need a $file package variable.

my %options = (title => '-t',
artist => '-a',
album => '-A',
tracknumber => '-T');

sub run_id3v2 (@)

You don't really need the (@) prototype as that is the default behavior
for subs.

{
my @command = ("id3v2", @_, $file);

Without the $file package variable this would be:

my @command = ( 'id3v2', @_ );

return system(@command) == 0;
}

foreach (@ARGV)
{
$file = $_;

foreach my $file ( @ARGV ) {

my @output = `vorbiscomment -l \Q$file\E 2>&1`;

You could use chomp() here instead of later in map.

chomp( my @output = `vorbiscomment -l \Q$file\E 2>&1` );

if ($? != 0)
{
print "Problems reading tags from $file, skipping it:\n", @output;
next;
}

my %comments = map { chomp; split /=/, $_, 2 } @output;

Without the chomp() this would become:

my %comments = map split( /=/, $_, 2 ), @output;


However, it looks like you are not using anything after the '='.

my @comments = map /^([^=]+)/, @output;

my @arguments = ();

# Also considered using (if this is strange indentation, thank
# cperl-mode):
#
# push @arguments,
# map {($options{$_}, $comments{$_})}
# grep exists $options{$_}, keys %comments;

push @arguments,

You are declaring @arguments and then pushing values into it. Why not
just assign the values when you declare it?

map {exists $options{$_} ? ($options{$_}, $comments{$_}) : ()}
keys %comments;

my @arguments =
map {exists $options{$_} ? ($options{$_}, $_) : ()}
@comments;

$file =~ s/\.ogg/.mp3/;

You probably only want to match /\.ogg/ if it is at the end of $file.

$file =~ s/\.ogg$/.mp3/;

run_id3v2('-D') or die "Problem removing old tags from $file.";
run_id3v2(@arguments) or die "Problem adding new tags to $file.";
run_id3v2('-s') or die "Problem removing new id3v1 tags from $file.";
}

With $file now local to the foreach loop you need to add it to the
argument list.

run_id3v2( '-D', $file ) or die "Problem removing old tags from
$file.";
run_id3v2( @arguments, $file ) or die "Problem adding new tags to
$file.";
run_id3v2( '-s', $file ) or die "Problem removing new id3v1 tags from
$file.";
}


HTH

John

 

[Robin]

sub run_id3v2 (@)

Do you even *try* before posting here? Gee, it's having to do with a
subroutine, you think *maybe* it might be in the documentation for
subroutines?

perldoc perlsub
it's called a prototype, although this particular one is rather useless.
Can you read the documentation and tell us why, Robin?

because he really isn't using any references? I scanned the perlsub docs,
and this is probably wrong, but I have to write a paper.
-Robin

 

[Paul Lalli]

Thanks, that's way more clear than the perldoc, in my humble opinion.

Basically to clarify your intitial question, this prototype doesn't do
anything because the arguments for a subroutine are already an array, @_,
correct?

On the right track, but your wording can quickly lead to incorrectness.
All subroutine's arguments are always passed into the array @_, regardless
of prototype. A prototype decides how many and what kind of arguments
will be allowed into that array. For example
sub f1($@);
declares f1 to be a subroutine that will accept a single scalar followed
by a list of values. Valid calls for this subroutine include:
f1($foo, @bar);
f1("hello", "world");
f1("hello", "how", "are", "you?");
f1(@arr1, $scal);

In this last case, the prototype would cause @arr1 to be automatically
referenced, and the remaining list would contain soley $scal. So in the
function @_ would contain two elements - a reference to @arr1, and an
alias to $scal.

If you read the section on prototypes, it will tell you what each
identifier means. $ is any scalar value, \$ is a named scalar variable, @
is a list of values, \@ is a named array variable, etc.

From this, you can see that @ is something of a catch-all, because in
Perl, any sequence of values is a list. In the same way that
(@foo, @bar) = (1..10);
will cause @foo to contain 1 through 10, and @bar to contain nothing,
sub foo($@@)
is nonsensical because the first list identifier already covers any
following values. In the general case then, it makes no sense for @ to
come anywhere but the last identifier in a prototype. In the specific
case of sub foo (@);, the prototype isn't doing anything special - it's
just saying "take a list of values" which is exaclty what all perl
subroutines do anyway.

Is that clearer, or did I just muddy the explanation?

One question, and I don't think this is in the perldoc perlsub, although I
still haven't fully covered it, but can a subroutine, with strict in use, be
called without parantheses in a script that's not a module after it has it's
prototype? Or that's not a subroutine imported from a module?

Parentheses have little to nothing to do with a prototype. However,
giving a subroutine a prototype requires that the compiler can see the
prototype before the subroutine is called. For example, this code
generates a warning:

foo ("bar", "baz");

sub foo(\@$){
print "In foo: $_[0], $_[1]\n";
}

In this case, the parser couldn't tell before you called foo() that foo()
had a prototype it should check. So it outputted a warning and continued,
ignoring the prototype.

You are, btw, allowed to predeclare subroutines before defining them:

sub foo(\@$);

foo ("bar", "baz");

sub foo (\@$){
print "In foo: $_[0], $_[1]\n";
}

This code will now generate a fatal error telling you you didn't properly
call foo(). The use of parentheses in calling the subroutine is optional,
because the subroutine was predeclared. (This is standard procedure for
all subroutines, and is unaffected by the use of prototypes).

I hope that's clear,
Paul Lalli

 

[Robin]

Almost correct, actually. A prototype of (@) tells perl "Only accept a
list of arguments" (as opposed to something like ($\@), which says "accept
a scalar value followed by an actual named array". But accepting a list
of arguments is the default behavior for Perl subroutines. Therefore,
using a prototype of (@) is no different than not using a prototype at
all. (it is very different, however, from using an empty prototype, like
sub myfunc() )

To clarify, prototypes do not have to have anything to do with references.
They can be used to restrict the number and type of arguments passed to
subroutines. For example:

sub myfunc($$) {

This function will only accept two scalar values. They can be any scalar
values you want, be them strings literals, numeric literals, named scalar
variables, or yes, references. But the following function calls are all
permitted:

myfunc("foo", 'bar');
myfunc(1, $string);
myfunc('', 0);

none of which do any conversions to references. In this case, the
prototype could be in place to cause fatal errors for any of these
function calls:

myfunc ("foo", "bar", "baz");
myfunc (42);
myfunc ();
@ten = (1..10);
myfunc (@ten);


Paul Lalli

Thanks, that's way more clear than the perldoc, in my humble opinion.

Basically to clarify your intitial question, this prototype doesn't do
anything because the arguments for a subroutine are already an array, @_,
correct?

One question, and I don't think this is in the perldoc perlsub, although I
still haven't fully covered it, but can a subroutine, with strict in use, be
called without parantheses in a script that's not a module after it has it's
prototype? Or that's not a subroutine imported from a module?

Thanks,

 

[Ben Morrow]

sub f1($@);
declares f1 to be a subroutine that will accept a single scalar followed
by a list of values. Valid calls for this subroutine include:
f1($foo, @bar);
f1("hello", "world");
f1("hello", "how", "are", "you?");
f1(@arr1, $scal);

In this last case, the prototype would cause @arr1 to be automatically
referenced, and the remaining list would contain soley $scal. So in the
function @_ would contain two elements - a reference to @arr1, and an
alias to $scal.

Not true. A prototype of $ causes that argument to be evaluated in
scalar context, i.e.

f1(@arr1, $scal);

is equivalent to

f1(scalar(@arr1), $scal);

which means that the first argument will be the *length* of the array,
not a ref to it.

Ben

 

[Paul Lalli]

Not true. A prototype of $ causes that argument to be evaluated in
scalar context, i.e.

f1(@arr1, $scal);

is equivalent to

f1(scalar(@arr1), $scal);

which means that the first argument will be the *length* of the array,
not a ref to it.

Whoops. You are correct of course. My apologies for the faulty
information. I should double check what I type when trying to inform
someone.

Paul Lalli

 

[Trebor A. Rude]

You don't really need the (@) prototype as that is the default behavior
for subs.

Ok, then. Probably just my C++ habits coming through.

Without the $file package variable this would be:

my @command = ( 'id3v2', @_ );

I noticed you changed to single quotes from double quotes. Is it common to
use single quotes unless you need interpolative context? Again, probably my
C++ habits.

You could use chomp() here instead of later in map.

chomp( my @output = `vorbiscomment -l \Q$file\E 2>&1` );

That probably works better, or at least makes the map a little easier to
understand. It seems stange to me to put a function around a variable
declaration, but that's just my C++ background yet again.

if ($? != 0)
{
print "Problems reading tags from $file, skipping it:\n", @output;
next;
}

my %comments = map { chomp; split /=/, $_, 2 } @output;

Without the chomp() this would become:

my %comments = map split( /=/, $_, 2 ), @output;


However, it looks like you are not using anything after the '='.

my @comments = map /^([^=]+)/, @output;

Now I'm confused. I was using the values of the %comments hash (which is
everything after the =, as the keys are everything before it) that's why
the map below contains $comments{$_}. In addition to that, I don't
understand your map statement. As I understand it, map returns a list which
is the result of applying the expression which is its first argumet to
every element of the list that is its second argument. But as far as I can
tell, the expression "/^([^=]+)/" doesn't do anything, leaving @comments as
an expensive copy of @output. Am I missing something, or did you mis-type
this statement?

You are declaring @arguments and then pushing values into it. Why not
just assign the values when you declare it?


my @arguments =
map {exists $options{$_} ? ($options{$_}, $_) : ()}
@comments;

The reason I didn't set as the same time as delcaration was probably due to
my trying to decide which of the two statements to use (though I guess they
both could been declarations).

I still don't see how your map statement produces the same result as mine
(but since I didn't understand your earlier map, maybe it's no surprise I
don't understand this).

At this stage of the program, %comments should look as though it were
declared like this:

%comments = (title => 'Untitled Hymn', artist => 'Chris Rice', album => 'Run
the Earth, Watch the Sky');

My map statement iterated over the keys from %comments, and if that key also
exists in %options, it would produce a list which was the value from
%options (so 'title' would go to '-t') and the value from %comments
(everything after the = in one of the lines of @output). That list
represents the arguments to the id3v2 program that are necessary to set a
tag in the mp3 file to the same value as the tag in the ogg file. I don't
see how you're duplicating that.

You probably only want to match /\.ogg/ if it is at the end of $file.

$file =~ s/\.ogg$/.mp3/;

You're right, forgot that.

Thanks for the advice so far. I hope we can work out our misunderstading of
the map issues.

Trebor

 

[Brian McCauley]

To clarify, prototypes do not have to have anything to do with references.

I do not think that that statement is a clarification.

s/not/not nececessarily/

They can be used to restrict the number and type of arguments passed to
subroutines.

In doing so the can also put an implicit take-a-reference in the call.

Given the prototype:

sub foo(\@);

foo(@q);

Is equivalent to

&foo(\@q); # & suppresses prototype

sub myfunc($$) {

This function will only accept two scalar values. They can be any scalar
values you want, be them strings literals, numeric literals, named scalar
variables, or yes, references. But the following function calls are all
permitted:

myfunc("foo", 'bar');
myfunc(1, $string);
myfunc('', 0);

As is

myfunc( @p, @q );

But it's equivalent to

&myfunc( scalar(@p), scalar(@q) );

--
\\ ( )
. _\\__[oo
.__/ \\ /\@
. l___\\
# ll l\\
###LL LL\\

 

[John W. Krahn]

my %comments = map { chomp; split /=/, $_, 2 } @output;

Without the chomp() this would become:

my %comments = map split( /=/, $_, 2 ), @output;

However, it looks like you are not using anything after the '='.

my @comments = map /^([^=]+)/, @output;

Now I'm confused. I was using the values of the %comments hash (which is
everything after the =, as the keys are everything before it) that's why
the map below contains $comments{$_}.

Yes, sorry, my mistake. Ignore that part of my advice.

In addition to that, I don't
understand your map statement. As I understand it, map returns a list which
is the result of applying the expression which is its first argumet to
every element of the list that is its second argument. But as far as I can
tell, the expression "/^([^=]+)/" doesn't do anything, leaving @comments as
an expensive copy of @output. Am I missing something, or did you mis-type
this statement?

No, it is typed correctly. The parentheses in a regular expression
capture their contents and return those contents in a list context which
map provides. To populate your hash you could use a regular expression
like this:

my %comments = map /^([^=]+)=(.+)/, @output;

Using that also means that you don't have to chomp() the line because .+
will not match the newline.


John

 

[Trebor A. Rude]

In addition to that, I don't
understand your map statement. As I understand it, map returns a list
which is the result of applying the expression which is its first argumet
to every element of the list that is its second argument. But as far as I
can tell, the expression "/^([^=]+)/" doesn't do anything, leaving
@comments as an expensive copy of @output. Am I missing something, or did
you mis-type this statement?

No, it is typed correctly. The parentheses in a regular expression
capture their contents and return those contents in a list context which
map provides. To populate your hash you could use a regular expression
like this:

my %comments = map /^([^=]+)=(.+)/, @output;

Using that also means that you don't have to chomp() the line because .+
will not match the newline.

Ok, I see now, thanks for the reminder. At this point, the expression is
back to doing just what the split/chomp combo does, just less clearly. I
did take your advice and moved the chomp up the variable declaration, so
it's just split in the map now, and that's probably the way I'll leave it.
But it is good to know that the alternative exists for situations where
split might not be the right thing.

Trebor

 

[John W. Krahn]

No, it is typed correctly. The parentheses in a regular expression
capture their contents and return those contents in a list context which
map provides. To populate your hash you could use a regular expression
like this:

my %comments = map /^([^=]+)=(.+)/, @output;

Using that also means that you don't have to chomp() the line because .+
will not match the newline.

Ok, I see now, thanks for the reminder. At this point, the expression is
back to doing just what the split/chomp combo does, just less clearly. I
did take your advice and moved the chomp up the variable declaration, so
it's just split in the map now, and that's probably the way I'll leave it.
But it is good to know that the alternative exists for situations where
split might not be the right thing.

Yes, use split() to remove data that you don't want and a match operator
to extract data that you do want, for example:

my @numbers = split /\D+/, $line;

my @numbers = $line =~ /\d+/g;

The split removes non-digit characters and the match extracts digit
characters however there is a subtle difference. Assuming that $line
contains the string " 123 456 789 \n" then split will return the
list ( '', 123, 456, 789 ) and the match operator will return the list (
123, 456, 789 ).



John