[vbnet9]

sub addSecondary($$)
{
        my ($email, $secondary) = @_;

        open (MYFILE, 'registration.dat');
        while (<MYFILE>)
        {
                chomp;
                my $line = $_;
                if($line =~ /$email/)
                {
                        $line =~ s/nodata/$secondary/gi;
                }
        }
        close (MYFILE);
}

I am trying to find strings which contain $email and replace the nodata part with $secondary. This code works but isn't updating the file. The data is still the same. What am I missing?

[dsherohman]

vbnet9, on 07 May 2012 - 04:51 AM, said:

This code works but isn't updating the file. The data is still the same. What am I missing?

You've forgotten to write the changes to the file. Line 12 changes the text in memory, but there's nothing in the code you showed which alters the file on-disk in any way. But that's a little trickier than it sounds because it tends to cause problems if you modify a file as you're reading it, especially if the new content isn't the same length as the old content. Because of this, you pretty much have to write the changes out to a new file, then rename it to replace the original after you're done.

A couple other more general points:

- What Perl calls a "prototype" and what C calls a "prototype" are two completely different and almost totally unrelated things. You don't need to use them and, the vast majority of the time, you don't want to use them. So don't use them unless you know that you absolutely have to. (In a dozen years of using Perl professionally as my language of choice, I've never used prototypes in production code. That's how rare it is to actually need them.)

- You're using "open" in an older way which is considered less maintainable and less secure than newer ways of using it. Google "perl three-argument open" and "perl lexical filehandles" for more details.

- Normal Perl style for multiple-word identifiers is to use all_lowercase_with_underscores, not camelCase. This doesn't actually affect how the code behaves, of course, but it will help you fit in with the broader Perl community if that matters to you.

Taking all that into account, here's a new version of your code which uses current best practices and will update the file correctly (code is untested, but should work):

sub add_secondary
{
        my ($email, $secondary) = @_;

        open my $infile, '<', 'registration.dat' or die "Failed to open registration.dat: $!";
        open my $outfile, '>', 'registration.new' or die "Failed to open registration.new: $!";
        while (<$infile>)
        {
                chomp;
                my $line = $_;
                if($line =~ /$email/)
                {
                        $line =~ s/nodata/$secondary/gi;
                }
                print $outfile "$line\n";
        }
        close $outfile;
        close $infile;

        rename 'registration.new', 'registration.dat';
}

But, of course, there's no reason to chomp the line when you read it and then put a \n back on when you write it, and there are more Perlish idioms for a lot of this, so here's how I'd write it (again, untested but should work):

sub add_secondary {
  my ($email, $secondary) = @_;

  open my $infile, '<', 'registration.dat' or die "Failed to open registration.dat: $!";
  open my $outfile, '>', 'registration.new' or die "Failed to open registration.new: $!";
  while (<$infile>) {
    s/nodata/$secondary/gi if /$email/;
    print $outfile;
  }
  close $outfile;
  close $infile;

  rename 'registration.new', 'registration.dat';
}