The right way to avoid an error

diarmuid has asked for the wisdom of the Perl Monks concerning the following question:

Hi,

I wrote a quick and dirty script to play mp3's from a html index. I parses the file and cuts out the mp3's and if they match a pattern it plays them. However when matching the pattern perl complains that Use of uninitialized value in concatenation (.) at play.pl line 10, <INPUT> line 23. I think it is interpreting the . (as in .mp3) as the concat operator and is bombing out. I got around this as you can see by escaping the . but what's the correct way to fix this problem?

use strict;

$SIG{INT} = 'IGNORE';
open(INPUT,"index.html") || die "Cant open index.html\n";

while(<INPUT>){
  if(/^<a href=\"(.*)\">/){
    my $mp3 = $1;
    $mp3 =~ s/\./\\\./g;
    if ($mp3 =~ /$ARGV[0]/i){
      $mp3 =~ s/\\\./\./g;
      (my $print_mp3 = $mp3) =~ s/%20/ /g;
      print "Now Playing $print_mp3\n";
      `mpg123 -p none -b 500 -q $mp3`;
    }
  }
}

close INPUT;
[download]

p> Diarmuid

Comment on The right way to avoid an error Select or Download Code

Replies are listed 'Best First'.
Re: The right way to avoid an error by DrZaius (Monk) on May 18, 2001 at 20:38 UTC
Why not: `use strict; use IO::File; use constant INDEX_FILE => './index.html'; my $fh = IO::File->new(INDEX_FILE) or die INDEX_FILE, ":$!"; while(<$fh>) { if(m!<a href=["']([^"']+)!i) { my $mp3 = $1; system('mpg123', "-p none -b 500 -q $mp3") if($mp3 =~ m!$ARGV[0]!i); } }` [download] Or, even better (not perl, and uses filesystem instead): `find . -type f -iname '*.mp3' \| grep "pattern here" \| xargs mpg123 -p +none -b 500 -q` [download] have fun.	[reply] [d/l] [select]
Re: The right way to avoid an error by arturo (Vicar) on May 18, 2001 at 20:55 UTC
More terser (and check out HTML::LinkExtor, too): use URI::Escape; # or CGI.pm has an escape() function # also at the top of the script. my $pattern = quotemeta( lc( $ARGV[0] ) ); # in your loop if (my ($mp3) = /^<a href="([^"]+)/) { # $mp3 should now hold a URL if ($mp3 =~ /$pattern/i) { $mp3 = uri_escape($mp3); `mpg123 -p none -b 500 -q $mp3`; } } [download] Update ChemBoy caught an error (didn't assign to `$mp3` in a list context! Check those parens, kids!) HTH! `perl -e 'print "How sweet does a rose smell? "; chomp ($n = <STDIN>); +$rose = "smells sweet to degree $n"; other_name = rose; print "$oth +er_name\n"'` [download]	[reply] [d/l] [select]
Re: The right way to avoid an error by Asim (Hermit) on May 18, 2001 at 20:39 UTC
Looks basically OK to me. Larry has said in the past the the correct way is whatever gets the job done before you get fired... Having said that, you might want to investigate using the core module File::Basename, which will do the file/ext split for you w/o needing the regexp. ----Asim, known to some as Woodrow.	[reply]
Re: The right way to avoid an error by sierrathedog04 (Hermit) on May 18, 2001 at 23:39 UTC
There appears to be an error in your code. `$mp3 =~ s/\./\\\./g;` [download] inserts a backslash before the period in $mp3. But since $mp3 appears on the left side of a regexp that backslash will be a literal one. If `$ARGV[0]` includes a backslash, then inside the right-side of a regexp that backslash is a special character which merely turns the next character, but not the backslash itself, into a literal. Thus, `if ($mp3 =~ /$ARGV[0]/i){` [download] will always fail because anyfilename.mp3 will never appear in anyfilename\.mp3 Adding that backslash to $mp3 breaks the program.	[reply] [d/l] [select]
Re: The right way to avoid an error by AidanLee (Chaplain) on May 18, 2001 at 20:51 UTC
Your script appears to be complaining about `$ARGV[0]` not having been initialized. The error you found doesn't really have anything to do with the concatenation operator itself. It's merely saying that one of the variables in the concatenation has yet to be initialized. This happens when you run your script with the -w flag or "use warnings." It does this because you may have accidentally mispelled a variable name and rather than using the variable you expected, you're using an uninitialized variable which could cause wierd things to happen. Of course, "use strict" reduces that possibility as well (/me wondering what the implications of making this warning go away under use strict). Since it's complaining about `$ARGV[0]` i have to wonder: did you supply a command line argument to your script? All in all, it may be safer to use Getopt::Std to do your command line input.	[reply] [d/l] [select]
Re: Re: The right way to avoid an error by diarmuid (Beadle) on May 18, 2001 at 21:17 UTC
This isn't the problem as I ran the script with a fixed string in the regexp before putting in ARGV[0] and I was still getting the same errors	[reply]
Re: The right way to avoid an error by AidanLee (Chaplain) on May 18, 2001 at 20:53 UTC
it's also important to note that the s/// operator does not take a regular expression within it's second set of /'s. So escaping the (.) doesn't really do anything. Odd that it should make the warning go away.	[reply]