«

I have pre-made code that deletes a line from a flat file database. I
am trying to pass an argument through a link to give the name of the
line to delete.

Here is my database.txt
image020.jpg | 6/4/2004 | 10:19:59 | | rena

Here is how I use the link
delete.cgi?filedelete=$filename

Now here is what prints out
image020.jpg
File image020.jpg not found!

It is obvious that it is grabbing the argument because it prints out
image020.jpg. The thing I don't understand is why it is not matching
with the image020.jpg in the database.txt.

Here is my code
#!c:/perl/bin/perl

use strict;
use warnings;
use CGI;
use CGI::Carp qw(fatalsToBrowser);
my $q = new CGI;
print $q->header, $q->start_html('Delete File');

my $action = new CGI;
my @temp=split(/=/,$ENV{'QUERY_STRING'});
my $filedelete = $temp[1];
my $file = 'C:/Program Files/Apache
Group/Apache2/htdocs/quickbooks/database.txt';
my $found = undef;
my (@ary);
my $line;

open( OLD, $file ) or die "Couldn't open old file $file: $! \n";
print $temp[1];
foreach $line (@ary) {
chomp($line);
(my $filename, my $filedate, my $filetime, my $notes, my $user) =
split(/\|/,$line);
push( @ary, $line ) unless ( $filedelete eq $filename );
$found = 1 if ( $filedelete eq $filename );
}

close OLD or die "Couldn't close old file $file: $! \n";

unless ($found) {
print $action->h2("File $filedelete not found!");
exit;
}

open( NEW, ">$file" ) or die "Couldn't open new file $file: $!\n";
foreach (@ary) {
print NEW $_, "\n"
}

close NEW or "Couldn't close new file $file: $! \n";;

exit;

print $q->end_html;

On Fri, 4 Jun 2004, Mark Constant wrote:

> Here is my database.txt
> image020.jpg | 6/4/2004 | 10:19:59 | | rena
>
> Now here is what prints out
> image020.jpg
> File image020.jpg not found!
>
> It is obvious that it is grabbing the argument because it prints out
> image020.jpg. The thing I don't understand is why it is not matching
> with the image020.jpg in the database.txt.
<snip>
> (my $filename, my $filedate, my $filetime, my $notes, my $user) =
> split(/\|/,$line);
> push( @ary, $line ) unless ( $filedelete eq $filename );
> $found = 1 if ( $filedelete eq $filename );

You're splitting on the | only. But look at your database lines. The
elements are separated by " | ", not by "|". So the contents of $filename
are 'images020.jpg ', which is not string equal to 'image20.jpg'

I would suggest changing your split line to
split /\s+\|\s+/, $line;

(untested on my part, btw).

Paul Lalli

Quoth (Mark Constant):
> I have pre-made code that deletes a line from a flat file database. I
> am trying to pass an argument through a link to give the name of the
> line to delete.
>
> Here is my database.txt
> image020.jpg | 6/4/2004 | 10:19:59 | | rena
>
> Here is how I use the link
> delete.cgi?filedelete=$filename
>
> Now here is what prints out
> image020.jpg
> File image020.jpg not found!
>
> It is obvious that it is grabbing the argument because it prints out
> image020.jpg. The thing I don't understand is why it is not matching
> with the image020.jpg in the database.txt.
>
> Here is my code
> #!c:/perl/bin/perl
>
> use strict;
> use warnings;
> use CGI;
> use CGI::Carp qw(fatalsToBrowser);
> my $q = new CGI;
> print $q->header, $q->start_html('Delete File');
>
> my $action = new CGI;
> my @temp=split(/=/,$ENV{'QUERY_STRING'});

DON'T EVER DO THIS AGAIN. Use CGI.

> my $filedelete = $temp[1];

my $filedelete = $action->param('filedelete');

> my $file = 'C:/Program Files/Apache
> Group/Apache2/htdocs/quickbooks/database.txt';
> my $found = undef;

my $found;

> my (@ary);

my @ary;

> my $line;
>
> open( OLD, $file ) or die "Couldn't open old file $file: $! \n";

Don't put "\n" on the end of die messages.
Use lexical filehandles.
Specify the mode to open the file in, for safety.

open my $OLD, '<', $file
or die "can't open $file: $!";

> print $temp[1];
> foreach $line (@ary) {

for my $line (@ary) {

Create variables in the smallest possible scope.

> chomp($line);
> (my $filename, my $filedate, my $filetime, my $notes, my $user) =

my ($filename, $filedate, $filetime, $notes, $user) =

But anyway, why do you need more than

my ($filename) =

?

> split(/\|/,$line);

You are splitting on /\|/. This will leave whitespace all round your
filename. Try

.... = split /\s* \| \s*/x, $line;

> push( @ary, $line ) unless ( $filedelete eq $filename );
> $found = 1 if ( $filedelete eq $filename );

None of those parens are necessary.

> }
>
> close OLD or die "Couldn't close old file $file: $! \n";
>
> unless ($found) {
> print $action->h2("File $filedelete not found!");
> exit;

No. This will not correctly end the HTML.

> }
>
> open( NEW, ">$file" ) or die "Couldn't open new file $file: $!\n";
> foreach (@ary) {
> print NEW $_, "\n"
> }
>
> close NEW or "Couldn't close new file $file: $! \n";;
>
> exit;

Nor will this.

if ($found) {
open my $NEW, '>', $file or die "couldn't create $file: $!";
print $NEW "$_\n" for @ary;
}
else {
print $action->h2("File $filedelete not found!");
}

> print $q->end_html;

You *must* use locking in a situation like this, or you will end up with
a corrupted database. Something like:

use Fcntl qw/:flock/;

{
open my $OLD, '<', ...;
flock $OLD, LOCK_SH or die "can't acquire read lock: $!";
# read stuff
}
# $OLD closes here, lock is released

if ($found) {
open my $NEW, '>', ...;
flock $NEW, LOCK_EX or die "can't acquire write lock: $!";
# write stuff
}
# ditto for $NEW

Alternatively, you could open the file '+<' and then lock it LOCK_EX,
and truncate it rather than closing/reopening.

Ben

--
It will be seen that the Erwhonians are a meek and long-suffering people,
easily led by the nose, and quick to offer up common sense at the shrine of
logic, when a philosopher convinces them that their institutions are not based
on the strictest morality. [Samuel Butler, paraphrased]

(Mark Constant) writes:

> Subject: Delete a line out of a flat file database

See FAQ: "How do I [ ... ]delete a line in a file [...]?"

> I have pre-made code that deletes a line from a flat file database.

What do you mean by "pre-made"? You mean someone else gave you this
code. Give it back to them.

> Here is my code
> #!c:/perl/bin/perl
>
> use strict;
> use warnings;
> use CGI;
> use CGI::Carp qw(fatalsToBrowser);

Good start.

> my $q = new CGI;
> print $q->header, $q->start_html('Delete File');
>
> my $action = new CGI;

What? Two CGI objects? Why?

> my @temp=split(/=/,$ENV{'QUERY_STRING'});

If you are using CGI why access $ENV{'QUERY_STRING'} directly?

> my $filedelete = $temp[1];

Ever heard of a list slice? There's no need for @temp.

> my $file = 'C:/Program Files/Apache
> Group/Apache2/htdocs/quickbooks/database.txt';
> my $found = undef;

No need to explicitly initialise to undef.

> my (@ary);
> my $line;

You are suffering from premature declaration. Variables should
usually be declared where they are first used.

>
> open( OLD, $file ) or die "Couldn't open old file $file: $! \n";
> print $temp[1];
> foreach $line (@ary) {

Here is where you should have declared $line.

@ary is always empty here so this loop will never do anything.

I think you meant to say:

while (my $line = <OLD>) {

> chomp($line);
> (my $filename, my $filedate, my $filetime, my $notes, my $user) =
> split(/\|/,$line);

my can take a list of aguments so the above is more simply written:

my ($filename, $filedate, $filetime, $notes, $user) =
split(/\|/,$line);

> push( @ary, $line ) unless ( $filedelete eq $filename );
> $found = 1 if ( $filedelete eq $filename );

Having and if and an unless with the same constion makes me think
you'd be better off with a if-else.

> }
>
> close OLD or die "Couldn't close old file $file: $! \n";
>
> unless ($found) {
> print $action->h2("File $filedelete not found!");
> exit;

You forgot the end_html.

> }
>
> open( NEW, ">$file" ) or die "Couldn't open new file $file: $!\n";
> foreach (@ary) {
> print NEW $_, "\n"
> }

You have not considered that the server may be handling two requests
at the same time. You need some kind of file locking.

> close NEW or "Couldn't close new file $file: $! \n";;

You forgot the die;

>
> exit;

This is spurious.

> print $q->end_html;


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

In article <c9q3p1$9cb$>, Ben Morrow wrote:
>
> Quoth (Mark Constant):
>> I have pre-made code that deletes a line from a flat file database. I
>> am trying to pass an argument through a link to give the name of the
>> line to delete.
>>
>> Here is my database.txt
>> image020.jpg | 6/4/2004 | 10:19:59 | | rena
>>
>> Here is how I use the link
>> delete.cgi?filedelete=$filename
>>
>> Now here is what prints out
>> image020.jpg
>> File image020.jpg not found!
>>
>> It is obvious that it is grabbing the argument because it prints out
>> image020.jpg. The thing I don't understand is why it is not matching
>> with the image020.jpg in the database.txt.
>>
>> Here is my code
>> #!c:/perl/bin/perl
>>
>> use strict;
>> use warnings;
>> use CGI;
>> use CGI::Carp qw(fatalsToBrowser);
>> my $q = new CGI;
>> print $q->header, $q->start_html('Delete File');
>>
>> my $action = new CGI;
>> my @temp=split(/=/,$ENV{'QUERY_STRING'});
>
> DON'T EVER DO THIS AGAIN. Use CGI.
>
>> my $filedelete = $temp[1];
>
> my $filedelete = $action->param('filedelete');
>
>> my $file = 'C:/Program Files/Apache
>> Group/Apache2/htdocs/quickbooks/database.txt';
>> my $found = undef;
>
> my $found;
>
>> my (@ary);
>
> my @ary;
>
>> my $line;
>>
>> open( OLD, $file ) or die "Couldn't open old file $file: $! \n";
>
> Don't put "\n" on the end of die messages.

Why not? It isn't needed and I never do, but there isn't any reason not to.
Better would be to explain why it not needed than just "don't do this". In
fact, from my 5.8.0's 'perldoc -f die':

Equivalent examples:

die "Can't cd to spool: $!\n" unless chdir '/usr/spool/news';
chdir '/usr/spool/news' or die "Can't cd to spool: $!\n"

> Use lexical filehandles.
> Specify the mode to open the file in, for safety.

In this case, I don't think it is necessary. The file name is known and
"normal". For dealing with unkowns, I would agree. According to the faq for
open:

Use 3-argument form to open a file with arbitrary weird
characters in it,

open(FOO, '<', $file);

> open my $OLD, '<', $file
> or die "can't open $file: $!";
>
>> print $temp[1];
>> foreach $line (@ary) {
>
> for my $line (@ary) {
>
> Create variables in the smallest possible scope.
>
>> chomp($line);
>> (my $filename, my $filedate, my $filetime, my $notes, my $user) =
>
> my ($filename, $filedate, $filetime, $notes, $user) =
>
> But anyway, why do you need more than
>
> my ($filename) =
>
> ?
>
>> split(/\|/,$line);
>
> You are splitting on /\|/. This will leave whitespace all round your
> filename. Try
>
> ... = split /\s* \| \s*/x, $line;
>
>> push( @ary, $line ) unless ( $filedelete eq $filename );
>> $found = 1 if ( $filedelete eq $filename );
>
> None of those parens are necessary.

But they do add clarity and help make precedence more obvious.

>
>> }
>>
>> close OLD or die "Couldn't close old file $file: $! \n";
>>
>> unless ($found) {
>> print $action->h2("File $filedelete not found!");
>> exit;
>
> No. This will not correctly end the HTML.
>
>> }
>>
>> open( NEW, ">$file" ) or die "Couldn't open new file $file: $!\n";
>> foreach (@ary) {
>> print NEW $_, "\n"
>> }
>>
>> close NEW or "Couldn't close new file $file: $! \n";;
>>
>> exit;
>
> Nor will this.
>
> if ($found) {
> open my $NEW, '>', $file or die "couldn't create $file: $!";
> print $NEW "$_\n" for @ary;
> }
> else {
> print $action->h2("File $filedelete not found!");
> }
>
>> print $q->end_html;
>
> You *must* use locking in a situation like this, or you will end up with
> a corrupted database. Something like:
>
> use Fcntl qw/:flock/;
>
> {
> open my $OLD, '<', ...;
> flock $OLD, LOCK_SH or die "can't acquire read lock: $!";
> # read stuff
> }
> # $OLD closes here, lock is released
>
> if ($found) {
> open my $NEW, '>', ...;
> flock $NEW, LOCK_EX or die "can't acquire write lock: $!";
> # write stuff
> }
> # ditto for $NEW
>
> Alternatively, you could open the file '+<' and then lock it LOCK_EX,
> and truncate it rather than closing/reopening.
>
> Ben
>

On Fri, 4 Jun 2004, Kevin Collins wrote:
>
> In article <c9q3p1$9cb$>, Ben Morrow wrote:
<snip>
> > Don't put "\n" on the end of die messages.
>
> Why not? It isn't needed and I never do, but there isn't any reason not to.
> Better would be to explain why it not needed than just "don't do this". In
> fact, from my 5.8.0's 'perldoc -f die':
>
> Equivalent examples:
>
> die "Can't cd to spool: $!\n" unless chdir '/usr/spool/news';
> chdir '/usr/spool/news' or die "Can't cd to spool: $!\n"
>

Because putting "\n" suppresses useful information. Without a "\n", the
die function will output not only its argument string, but also the file
and line number on which the die occurred. Adding the "\n" in there makes
you lose this information.

<snip>
> >> push( @ary, $line ) unless ( $filedelete eq $filename );
> >> $found = 1 if ( $filedelete eq $filename );
> >
> > None of those parens are necessary.
>
> But they do add clarity

That's debatable.

> and help make precedence more obvious.

"Learn the language".


Paul Lalli

(Mark Constant) wrote in message news:< om>...
> I have pre-made code that deletes a line from a flat file database. I
> am trying to pass an argument through a link to give the name of the
> line to delete.
>
> Here is my database.txt
> image020.jpg | 6/4/2004 | 10:19:59 | | rena
>
> Here is how I use the link
> delete.cgi?filedelete=$filename
>
> Now here is what prints out
> image020.jpg
> File image020.jpg not found!
>
> It is obvious that it is grabbing the argument because it prints out
> image020.jpg. The thing I don't understand is why it is not matching
> with the image020.jpg in the database.txt.
>
> Here is my code
> #!c:/perl/bin/perl
>

[variable initialzation snipped]

>
> open( OLD, $file ) or die "Couldn't open old file $file: $! \n";
> print $temp[1];

The array @ary is empty. I think you are missing a line such as my @ary = <OLD>;

> foreach $line (@ary) {
> chomp($line);
> (my $filename, my $filedate, my $filetime, my $notes, my $user) =
> split(/\|/,$line);
> push( @ary, $line ) unless ( $filedelete eq $filename );
> $found = 1 if ( $filedelete eq $filename );
> }
>
> close OLD or die "Couldn't close old file $file: $! \n";
>
[rest of program snipped]

--
Jim Gibson

In article <>, Paul Lalli wrote:
> On Fri, 4 Jun 2004, Kevin Collins wrote:
>>
>> In article <c9q3p1$9cb$>, Ben Morrow wrote:
><snip>
>> > Don't put "\n" on the end of die messages.
>>
>> Why not? It isn't needed and I never do, but there isn't any reason not to.
>> Better would be to explain why it not needed than just "don't do this". In
>> fact, from my 5.8.0's 'perldoc -f die':
>>
>> Equivalent examples:
>>
>> die "Can't cd to spool: $!\n" unless chdir '/usr/spool/news';
>> chdir '/usr/spool/news' or die "Can't cd to spool: $!\n"
>>
>
> Because putting "\n" suppresses useful information. Without a "\n", the
> die function will output not only its argument string, but also the file
> and line number on which the die occurred. Adding the "\n" in there makes
> you lose this information.
>
><snip>
>> >> push( @ary, $line ) unless ( $filedelete eq $filename );
>> >> $found = 1 if ( $filedelete eq $filename );
>> >
>> > None of those parens are necessary.
>>
>> But they do add clarity
>
> That's debatable.

If you use parens, there can be no ambiguity...

>> and help make precedence more obvious.
>
> "Learn the language".

Well, then why bother with indenting, white-space, comments, etc? If you "know
the language", why use any of those constructs - they are mostly not required?

Kevin

>>>>> "KC" == Kevin Collins <> writes:

>>> >
>>> > None of those parens are necessary.
>>>
>>> But they do add clarity
>>
>> That's debatable.

KC> If you use parens, there can be no ambiguity...

>>> and help make precedence more obvious.
>>
>> "Learn the language".

KC> Well, then why bother with indenting, white-space, comments, etc?
KC> If you "know the language", why use any of those constructs - they
KC> are mostly not required?

then you might as well put parens around everything and become lisp.

parens are best used when needed and they can hide (or make noisy) stuff
when they are not. i still do push( @foo, $bar ) for style reasons and
not from any ambiguity thing. the code (and reader) should be expected
to know basic perl precedence rules. baby code is ok when you start out
but you should graduate to cleaner code where you assume the reader has
enough skill to follow basic perl.

uri

Paul Lalli wrote:

> On Fri, 4 Jun 2004, Kevin Collins wrote:
>
>>In article <c9q3p1$9cb$>, Ben Morrow wrote:
>>>Don't put "\n" on the end of die messages.
>>
>>Why not? It isn't needed and I never do
>
> Because putting "\n" suppresses useful information. Without a "\n", the
> die function will output not only its argument string, but also the file
> and line number on which the die occurred. Adding the "\n" in there makes
> you lose this information.

That additional information is completely useless to the end user.
1) Leave out the trailing \n for messages that tell the programmer
there is a bug in the program that needs to be fixed.
2) Put in the \n for messages that tell the user that he/she entered
something wrong and should try again.

-Joe