Re: Useless Program
by btrott (Parson) on Jun 01, 2000 at 09:56 UTC
|
That regexp shouldn't have the $ escaped. That makes it
search for the literal string '$name', not the value of $name.
Try this:
if (/^$name$/i) {
You may want to quote any meta-characters in the name:
if (/^\Q$name\E$/i) {
Your other, bigger problem, as cds pointed out, is
that you're only searching the first line. If the match isn't in
the first line, you then ask whether or not to add the name.
You'll want to fix this.
My suggestion, in fact, would be to use a DBM file. DB_File,
perhaps. | [reply] [d/l] [select] |
|
|
You're obviously right about the regex, but would you need it in the first place? If your file looks like
foo
bar
baz
and the input is "foo\n", why would you want to:
- chop input trailing \n
- chop each line's trailing \n
- regex on /^ AND $/??
am I missing something, or isn't it just easier to say $name eq $_ and leave it at that?
#!/home/bbq/bin/perl
# Trust no1!
| [reply] [d/l] |
|
|
Well...if you don't use ^ and $ it could match a name that includes the name you are searching for...for example if $name = 'bob' it would match bobbie, bobby, etc...
Just a thought...
--Amber
| [reply] |
|
|
| [reply] |
|
|
RE: Useless Program
by cds (Sexton) on Jun 01, 2000 at 09:55 UTC
|
You're problem would be that you only ever test the first line. If this is equal to the search term, then you say it was found. Otherwise, you're saying it wasn't, appending it, and exiting the loop.
Easiest solution would be have a loop that goes through checking each line. Have a flag that you set if you find the name (and exit the loop if you do). The after the loop check the flag. If it's set, you found the name and print the apropriate message. Otherwise you ask if they want to add it.
Colin Scott
If you build it, they will be dumb...
| [reply] |
Re: Useless Program
by httptech (Chaplain) on Jun 01, 2000 at 17:05 UTC
|
Also, since $name isn't going to change during the course
of the program, use the 'o' modifier to tell perl to compile
the regex once instead of every time it is evaluated. This
will speed up your search.
/^$name$/io
| [reply] [d/l] |
Re: Useless Program
by lhoward (Vicar) on Jun 01, 2000 at 17:12 UTC
|
You can make your scale better (to a file
with a large number of names) by employing some sort of DB.
In this case it also makes the code shorter.
Here's your program using a tied hash to a SDBM file.
All the modules you should need to do this are included with
the perl install.
use Fcntl;
use SDBM_File;
my %db_names;
tie (%db_names, 'SDBM_File', './names.db', O_RDWR | O_CREAT, 0666);
print "Enter your name--> ";
chomp (my $name = <STDIN>);
if(defined $db_names{$name}){
print "Your name was already found\n";
}else{
print "Name not found. Would you like to add it? (y/n)--> ";
chomp (my $yn = <STDIN>);
if ($yn eq 'y') {
$db_names{$name}=1;
}
}
untie %db_names;
| [reply] [d/l] |
Re: Useless Program
by c-era (Curate) on Jun 01, 2000 at 15:21 UTC
|
I don't have time to try this, but this code should work.
#!/usr/bin/perl -w
use strict;
open (FILE, "+<users.txt") or die "File not found\n"; #Open the file
+ in read/write mode
flock FILE,2; # It is good practice to lock a file you will write to
+.
my $name = '';
my $yn = '';
print "Enter your name--> ";
chomp ($name = <STDIN>);
while (<FILE>) {
chomp;
if (/^$name/) {
print "Your name was already found\n";
exit; }
}
print "Name not found. Would you like to add it? (y/n)--> ";
chomp ($yn = <STDIN>);
if ($yn eq 'y') {
print FILE "$name\n"; #Since we went through the file we can n
+ow print to the end of the file
close FILE; #Closing a file will unlock it
}else {
print "FINE!\n"; ;
}
print "end.\n";
| [reply] [d/l] |
|
|
One problem with this code is if you're adding a name that is the
prefix of a name already in the DB it will think that it is alrady there.
Try running this code and adding les after leslie has
already been added. Better to use eq instead of
a regular expression in your compare since you are looking
for an exact match. It will be faster too:
if ($_ eq $name) {
instead of
if (/^$name/) {
| [reply] [d/l] [select] |
|
|
Should you be holding a file lock around user input? I was always taught to lock a file only when you aren't potentially waiting the for the user to type something. Perhaps the FLOCK should be moved to right before the WHILE loop (and perhaps you only want to lock it when you know you need to write to it .. in that case, move the FLOCK to precede the 'print FILE' statment).
| [reply] |
|
|
You have to be careful there... You're currenly only explicitly closing the file if they choose "y". Move the close FILE; to outside of the if/then/else statement for best results... (perhaps not a big deal, but I've found that if I'm not careful with my file closing, my server gets cranky...)
| [reply] |
RE: Useless Program
by Anonymous Monk on Jun 01, 2000 at 17:51 UTC
|
At the bottom is a version that works. I found two bugs.
The first is that you complain about "name not found" inside
the while loop. This means that you complain if the name
doesn't happen to be the first one on the list. Moving
the complaint outside of the while loop causes it to only
fire off if _none_ of the names match, which is presumably
what you want.
The second bug is in the regexp. The backslash messes it
up. I would have expected it to match the literal string
"$name", but it doesn't seem to. Without the backslash, it
matches on a line containing exactly the value of the
$name variable.
#!/usr/bin/perl -w
use strict;
open (FILE, "users.txt") or die "File not found\n";
my $name = '';
my $yn = '';
print "Enter your name--> ";
chomp ($name = <STDIN>);
my $nameWasFound = 0;
while (<FILE>) {
if (/^$name$/i) {
print "Your name was already found\n";
$nameWasFound = 1;
last;
}
}
if(!$nameWasFound){
print "Name not found. Would you like to add it? (y/n)--> ";
chomp ($yn = <STDIN>);
if ($yn eq 'y') {
close FILE;
open (OUTPUT, ">>users.txt");
print OUTPUT "$name\n";
close OUTPUT; ;
}
else {
print "FINE!\n"; ;
}
}
print "end.\n";
| [reply] |
RE: Useless Program
by Anonymous Monk on Jun 01, 2000 at 20:21 UTC
|
Try this:
#!/usr/bin/perl -w
use strict;
my( $name, $yn, @usrlst );
#read the file
open( FILE, "<users.txt" ) || die;
@usrlst = <FILE>;
close( FILE );
print "Enter your name--> ";
chomp( $name = <STDIN> );
foreach( @usrlst ) {
if( /$name/igmo ) {
print "That name has already been added";
last;
}
else {
print "Name not found. Would you like to add it? (y/n)--> ";
chomp( $yn = <STDIN> );
if($yn =~ /y/i ) {
open( OUTPUT, ">>users.txt" );
print OUTPUT $name, "\n";
close OUTPUT;
} else { print "FINE, be that way!\n"; }
}
}
print "And the boredom continues.\n";
Don't know if it'll compile...just jotted it down here in the text
editor. G'luck. | [reply] |
RE: Useless Program
by seana (Initiate) on Jun 01, 2000 at 21:45 UTC
|
As I recall, there is a similiar program in Learning Perl by Schwartz... There he explains the nuances of this type of perl program. | [reply] |
RE: Useless Program
by seana (Initiate) on Jun 01, 2000 at 17:19 UTC
|
As I recall, there is a similiar program in Learning Perl by Schwartz... There he explains the nuances of this type of perl program. | [reply] |
Re: Useless Program
by Punto (Scribe) on Jun 01, 2000 at 17:24 UTC
|
Also, I guess it'd be faster if you chop the last character of the line ("\n") and then compare with the name, like this:
#open the file
while ($name_on_file = <FILE>) {
chop $name_on_file;
if ($name eq $name_on_file) {
#found the name
exit;
};
};
#ask for the name, and everithing
| [reply] [d/l] |
|
|
chop should be changed to chomp, as chomp is safe, and will only chop the record seperator. ( newline, unless you changed the value of $/ )
Paris Sinclair | 4a75737420416e6f74686572
pariss@efn.org | 205065726c204861636b6572
I wear my Geek Code on my finger.
| [reply] |