grep problems and file copying issues

[Viking]

Grep seems to be matching every line irrelevant of $cdLabel, what am I doing wrong? Is grep the best thing to use anyway?

Also what is the best way to make a backup of a file? ie: just making a copy of the file. I want to make this code portable so using the external cp command isn't an option. And my way of doing it works fine but is there a better way? I would like this script to be completely self reliant so a module isn't an option either.

open DBFILE , "$dbFileName" or die $!;
my @dbFile = <DBFILE>;
close DBFILE;

if (grep $cdLabel, @dbFile) {
  print "This CD ($cdLabel) is already in the database\n";
  exit; 
}

# `cp -f $dbFileName $dbFileName.BAK`;

open DBFILEBK, ">$dbFileName.BAK" or die $!;
for (@dbFile) { print DBFILEBK; }
close DBFILEBK;

[davorg]

You need to re-read the documentation for grep. What it does is applies your expression to each element in the list in turn and returns a list containing the elements for which your expression returns a true value. Each time round, you evaluate the expression $cdLabel. Presumably this always has a value which evaluates as true and therefore all elements of the list appear in the new list.

What you probably want to do is to create a regular expression to check if the value of $cdLabel is equal to the value of the element in $_. Something like /$cdLabel/ would be a simple solution, but that might cause problems if one CD title can be contained within another one. Perhaps using regexes isn't the best approach and you should just use $_ eq $cdLabel.

Oh... and for the copying question, use the File::Copy module. I know you didn't want to use a module, but it is part of the standard distribution so you should be able to rely on its presence.

--
<http://www.dave.org.uk>

European Perl Conference - Sept 22/24 2000, ICA, London
<http://www.yapc.org/Europe/>

[reptile]

The first argument to grep should be either an expression or a block of code, which evaluates to true or false depending on the value grep sets to $_ for each of the array. In other words, what you want is something like:

    grep(/$cdLabel/o, @dbFile);
    # or the equivilant in a code block
    grep { /$cdLabel/o } @dbFile;

Since you're using the grep EXPR, ARRAY version of grep, it's trating $cdLabel as an expression which will always evaluate to true (unless of course $cdLabel contains a false value).

As for creating the backup, there doesn't seem to be anything wrong with the way you're doing it. However, you could look into the Shell module (comes in the standard distribution) which enables a set of shell commands portably within perl (like echo, cp, ps, etc), and then just cp($dbFileName, "$dbFileName.BAK");

But to each his own. What you have will work just fine.

local $_ = "0A72656B636148206C72655020726568746F6E41207473754A";
while(s/..$//) { print chr(hex($&)) }

[le]

This one should work:

open (TEST, "test.txt") or die $!;

@array = <TEST>;

for (grep /$seachstring/, @array) {
        print;
}

[chromatic]

grep'll do what you want, in this case, but I would rather use a hash. (As your list of CDs grows, the grep operation will take longer, while a hash lookup will always take the same amount of time.)

my %dbfile = ();
open DBFILE , "$dbFileName" or die $!;
foreach (<DBFILE>) {
    $dbfile{$_} = 1;
}
close DBFILE;

if (defined $dbfile{$cdLabel})
    print "This CD ($cdLabel) is already in the database\n";
    exit; 
}

Better way to do it.

[ambiguous]

Good answer. I'd just like to add don't forget that hash keys are case sensitive so be to lc() or uc() they keys when you populate and reference the hash. Also, depending on how/where the $cdLabel is populated, you should probably strip all leading and trailing white space, $cdLabel =~ s/^\s+//; $cdLabel =~ s/\s+$//; Hope that helps.