Lou
Lou

Reputation: 2519

Perl - Uninitialised variable warning

I'm struggling to work out what I'm doing wrong here.

The goal of my code is to read a file, movie_script.txt, and then use regexes to sort each line into an array based on the character speaking the line. It does work, but I'm getting output preceded by these warnings:

Use of uninitialized value $char in string eq at filter.pl line 24, <$fh> line 13.
Use of uninitialized value $char in string eq at filter.pl line 26, <$fh> line 13.
Use of uninitialized value $char in string eq at filter.pl line 28, <$fh> line 13.
[...]
 Hello, mother.
 Oh. Well-- well, I, uh--
 Well, uh, I think they must have popped by for something.
 Mm, they-- they started following me yesterday.

Here's the code:

use strict;
use warnings;

my $filename = "movie_script.txt";

unless (-e $filename) {
    print "Error: File does not exist.";
}

my @brian;
my @mandy;
my @followers;

open(my $fh, '<', $filename);

my $match = qr/^(\w+):(.+)$/i;

while (my $line = <$fh>) {
    my $char = "";
    my $scriptline = "";
    if ($line) {
       ($char, $scriptline) = $line =~ $match;

        if ($char eq "BRIAN") {
            push(@brian, $scriptline);
        } elsif ($char eq "MANDY") {
            push(@mandy, $scriptline);
        } elsif ($char eq "FOLLOWERS") {
            push(@followers, $scriptline);
        } else {
            print($line);
        }
    }
}

foreach (@brian) {
    print "$_\n";
}

I suspect that the problem is a line doesn't fit my regex, and it's causing problems for the variables $char and $scriptline, but I don't know how to confirm whether that's true, or how to find out which line is causing the problem.

I've tried running the Perl debugger using perl -d, but when I proceed through each line, I can't find the error. I've tried to set a breakpoint around the `else { print($line) } line, but I can't work out how to run the debugger until it reaches that line.

Is there an obvious reason in my code why I'm getting an uninitalised value problem?

Upvotes: 1

Views: 77

Answers (4)

Polar Bear
Polar Bear

Reputation: 6818

Following perl script

  • takes a filename for processing or use default filename `movie_script.txt'
  • looks in the file for pattern CHARACTER: LINE and fills hash with data
  • sorts characters and prints their lines
use strict;
use warnings;
use feature 'say';

my $filename = shift || 'movie_script.txt';
my $match = qr/^(\w+):(.+)$/i;
my %script;

open my $fh, '<', $filename
    or die "Couldn't open $filename";

while(<$fh>) {
    next if /^\s*\Z/;
    push @{$script{$1}}, $2 if /$match/;
}

close $fh;

for my $char ( sort keys %script ) {
    say $char;
    say "\t$_" for @{$script{$char}};
}

Output

BRIAN
         Oh. Well-- well, I, uh--
         Well, uh, I think they must have popped by for something.
         Mm, they-- they started following me yesterday.
FOLLOWERS
         The Messiah! The Messiah! Show us the Messiah!
         The Messiah!
         The Messiah! The Messiah!
         Show us the Messiah! The Messiah! The Messiah! Show us the Messiah!
MANDY
         Don't you 'hello mother' me. What are all those people doing out ther    e?!
         Come on! What have you been up to, my lad?!
         'Popped by'?! 'Swarmed by', more like! There's a multitude out there!
         Well, they can stop following you right now. Now, stop following my son! You ought to be ashamed of yourselves.
         The who?
         Huh, there's no Messiah in here. There's a mess, all right, but no Me    ssiah. Now, go away!
         Ooooh.
         Now, you listen here! He's not the Messiah. He's a very naughty boy! Now, go away!
RIAN
         Hello, mother.

Upvotes: 0

TLP
TLP

Reputation: 67930

The reason for your uninitialized warnings is that some lines in your file do not match the regex, but you still use the variables. The solution is to not use the variables when the regex does not match by checking with an if statement as shown in the example below.

A few quick points.

  1. Consider using a hash instead of a number of arrays to store your lines. This will make the script re-usable and flexible.
  2. It is unnecessary to hard code a file name. You can simply supply the file name on the command line, and use the diamond operator <> to read the file: while (my $line = <>) { ....

And run it with

$ lines.pl movie_script.txt
  1. You do not need to add variables for the things you match with your regex. That is already taken care of since you use parentheses () in your regex. The matches are stored in pre-defined variables $1 and $2.

  2. You capture leading spaces in your lines, which can be fixed by adding \s* in the regex in front of the second parenthesis.

  3. You are using the /i modifier in your regex, which is unused, since you do not have any letters in your regex. (E.g. if you did /foo/i it would match FOO)

  4. You can use the /s modifier to allow .+ to match the newline, so you do not have to add it back later.

Your program is a very basic, typical perl one-liner, and it doesn't need to be complicated. In this example, I am using the Data::Dumper module to show you what the resulting data structure looks like:

use strict;
use warnings;
use Data::Dumper;

$Data::Dumper::Useqq = 1;           # show newline in Dumper print

my %lines;
my $match = qr/^(\w+):\s*(.+)/s;    # remove unused /i, add /s

while (my $line = <DATA>) {
    if ($line =~ $match) {          # Check before you use the variables
        push @{$lines{$1}}, $2;     # Store the matches in your hash
    }  # <--- if you want to do something with unmatched lines, use else
}

print for (@{$lines{BRIAN}});
print Dumper \%lines;

__DATA__
BRIAN: Hello, mother.
MANDY: Hi
BRIAN: Oh. Well-- well, I, uh--
FOLLOWERS: Hello

(I used the <DATA> filehandle to simulate your text file inside my program, just replace <DATA> with <>)

This program will output

Hello, mother.
Oh. Well-- well, I, uh--
$VAR1 = {
          "BRIAN" => [
                       "Hello, mother.\n",
                       "Oh. Well-- well, I, uh--\n"
                     ],
          "MANDY" => [
                       "Hi\n"
                     ],
          "FOLLOWERS" => [
                           "Hello\n"
                         ]
        };

It will work with different file names, it will capture lines of any named characters, not just the 3 you hard coded.

Upvotes: 1

Dave Cross
Dave Cross

Reputation: 69314

Consider asking Perl to tell you what the problem is.

if ($line) {
   if (my ($char, $scriptline) = $line =~ $match) {
     # Your existing code here
   } else {
     warn "Line [$line] doesn't match the regex [$match]\n";
   }

Note I've also moved the declaration of $char and $scriptline into the smallest possible scope. There's no point in declaring them any earlier or pre-populating them (as you're going to overwrite that data in the match line).

Upvotes: 2

Wander Nauta
Wander Nauta

Reputation: 19695

If you've got lines in the (movie) script that do not have a character speaking them, your regex will not match, and $char and $scriptline will not have values. You will want to skip those lines in some way.

There's many ways to do that, but one way would be to move the match operator to the if condition:

if (($char, $scriptline) = $line =~ $match) {
    if ($char eq "BRIAN") {
        push(@brian, $scriptline);
    } elsif ($char eq "MANDY") {
        push(@mandy, $scriptline);
    } elsif ($char eq "FOLLOWERS") {
        push(@followers, $scriptline);
    } else {
        print($line);
    }
}

The script will now ignore all lines that aren't dialog, push lines spoken by these specific cast members, and print lines spoken by others.

Upvotes: 2

Related Questions