sub duplicate_record {
   my ($name, $surename) = @_;
   my $is_duplicate = 0;
   my $html_title = 'Duplicate records have been found!';

   open (DBFILE, "dbfile.pdb") or die("Cannot open file for matching: 
+$!");
   local $_;
   while (<DBFILE>) {
      my @pattern_check = split(/,/, $_); #spit line where you see com
+ma (,) ($record)
      if ($name eq $pattern_check[0] and $surename eq $pattern_check[1
+]) { #check if duplicate is available
         print $cgi->start_html($html_title).
               $cgi->h1('Duplicate records:').
               $cgi->p("$name $surename $phone already exists.\nGo bac
+k and fill something else than that!").
               $cgi->end_html;
         $is_duplicate = 1;
         last;
      }
   }
   close (DBFILE);

   save_db() unless $is_duplicate;
}
[download]

This should work better, let me make some remarks

• Don't use global variables, pass the relevant variables ($name and $surename) to your subroutine.
• use strict;
• put the important part first: open or die and not die unless open.
• use last, next and redo for loop control
• Readup how <FILE> behaves in scalar and list context.
• Think about better variable names: $found -> $is_duplicate. What does $name stand for? $firstname?

• As gjb points out, there might be a problem with not printing a HTTP header, too. I assumed in my snippet that this header is printed somewhere else before calling the sub.

-- Hofmator

This looks like seriuous need for rewrite and refactoring. I'll add explanations as comments:

# for maintainability's sake:
#   keep this filename in one place
our $database_file = 'dbfile.pdb';

# self-explanatory name
sub add_person {
    my ($name,$surname,$phone) = @_;

    # a fully qualified call to open
    open my $dbfile, '+<' , $database_file
         or die "Error opening '$database_file': $!";

    flock $dbfile,LOCK_SH #share this file for now
         or die "Error flocking '$database_file': $!";

    # $dbfile is what was DBFILE before, but lexikals are
    #   less dangerous, because the handle is not global
    #   and you needn't locale()ize the *GLOB

    # using a while loop here
    #  note explicit localization of $_!

    # you had two loops around the handle, which was
    #  nonsense. This here says: while you can read data 
    #  from $dbfile

    # a flag indicating a duplicate was found;
    my $dupe = 0;

    while(defined(local $_ = <$dbfile> )){

        # source of errors: don't forget to chomp!
        chomp;

        # use usefull variable names
        my ($check_name,$check_surname) = split /,/;

        if( $name    eq $check_name and
            $surname eq $check_surname
           ){ # huston, we have a duplicate
           ++ $dupe;
           # so we can stope here
           last;
        }
    } # that's it

    if( $dupe ){
        close $dbfile;
        return; #false, this means: the name was already in
                # the database
    }
    else {
        # now we'll write to the file, so let's lock it
        flock $dbfile,LOCK_EX
              or die "Couldn't get excl. lock ... : $!";

        # go to the end of the file
        seek $dbfile , 0 , 2;

        # and add the new dataset
        print $dbfile "$name,$surname,$phone\n";

        close $dbfile
              or die "Write error in '$database_file': $!";

        return 1; #success
    }
}
[download]

The code that generates output should be in another function that only invokes add_person() and uses the return value as indicator for success. Keep frontend and backend seperate! The exceptional errors (all the die - cases) must be caught differently.

You built two loops around the <DBFILE> statement, which caused the outer loop (while) to be executed exactly once, stripping the first line from the file stream and the second loop (foreach) to iterate over all following lines, btw. loading them all into memory at once, which is never a good idea. The inner loop would only finish after all lines of the file were read, which would cause the outer loop to stop, too.

In the while the current line (i.e. record) gets assigned to $_, while in the foreach you read all lines (i.e. records) at once. I don't think that's what you really want. Leaving out the foreach and splitting $_ rather than $record seems more like it.

However, this probably has nothing to do with your problem. In the code you show I don't see a HTTP header printed (print $cgi->header();. Omitting a header may cause the fact that nothing seems to show up in the browser.

Hope this helps, -gjb-

$cgi-> p("$name $surename $phone already exists.\nGo back and 
+fill something else than that!").
[download]

It would be much better if you didn't use variables declared outside your sub, but instead passed them in as parameters (the technical term is 'coupling', iirc) - it makes for better long-term maintainability.

rdfield

#!/usr/local/bin/perl -w

use strict;

# Read in file and build HoH
open INPUT, "<names.csv" or die "Blerch: $!\n";
my %existing_names;
foreach $line (<INPUT>) {
  my ($name,$surname)=split /,/, $line;
  $existing_names{$surname}{$name}++;
}  
close INPUT;
...
# Check and add new record
sub duplicate_record {
  
  my $name=shift;
  my $surname=shift;

  if (exists ($existing_records{$surname}{$name})) {
    do_complain_about_duplicates();
    return;
  }
 
  $existing_records{$surname}{$name} 
  
}
[download]

open OUTPUT, ">names.csv" or die "Hcrelb: $!\n";
foreach my $surname (keys %existing_names) {
   print OUTPUT "$_,$surname\n" for keys %{$existing_names{$surname}};
}
close OUTPUT;
[download]

CU
Robartes-

      while(<DBFILE>){
         chomp;
         my @pattern_check = split(/,/,$_);
         .....
      }
[download]