>>>>> "sw" == shawn wilson <[email protected]> writes:
sw> 1. what's the reason for 'use warnings' instead of 'perl -w'? just
sw> readability or is there something more?
use warnings are lexical and affect only the file/scope they are in. -w
is a global flag that affect all the code. if you use a module that
isn't warnings clean then -w will warn about it but use warnings in your
code won't
sw> 2. i read the docs you referenced @ARGV on but i didn't see any
sw> reference to it. granted, i haven't googled this further, however
sw> what's the reason for not directly referencing it? the same reason
sw> as not referencing $_? - readability?
you didn't get it. directly using @ARGV is fine. processing it for
standard options with your own code is bad. use a standard options
module for that as it works, does more than you can and is easier to
use. and if there are args left in @ARGV you can still get at them. if
your program just takes a few args without options, then directly using
@ARGV is fine.
using $_ is not a good idea for several reasons, including
readability. it makes hard to track things when used in a nested
loop. it can lead to action at a distance bugs.
sw> 3. i'm aware of the sql injection possibilities with this code and
sw> though i have no intention of using placeholders (it's a PITA) i
sw> will do verbose checking on my strings as well as looking at the
sw> dbd taint option. however,
that isn't good enough. placeholders are about more than just
security. it makes your code easier to manage and maintain.
sw> i don't understand what you mean here:
sw> "You are preparing similar statements time and again. Prepare it once and
sw> re-
sw> use it with different parameters."
sw> can you rephrase that or give me an example of what you mean?
you can call prepare ONE time for a given sql statement with
placeholders. then you can call that statement handle many times, each
with possibly different arguments. it becomes like an sql sub which you
reuse.
sw> 4. Per this:
sw> "Moreover, what are you trying to do in these lines:"
sw> i'm trying to see if the word is defined in that hash, which was erroring,
sw> so i removed it.
sw> $data->{ $aword } = 0 unless defined( $data->{ $aword } );
sw> if it's not, define it. then make define the value to the below to 'line'.
sw> $data->{ $aword }->{ $line } = 'line';
sw> ... it was supposed to be the word line and not the variable for me to
check
sw> against later.
$data->{ $aword } ||= 0 ;
that will put 0 in there if it is false (undef, 0 or ''). should be fine
for you. recent perl's have // which is a defined/or and would be better
if '' is a legal value for you.
sw> my $dbh = DBI->connect('DBI:mysql:db;host=localhost',
sw> 'user', 'pass')
sw> or die "Database connection: $!";
sw> open( FILE, "< $ARGV[0]" );
why do you check the dbi connect for failure and not the file open?
always check open calls for failure.
sw> my $data;
sw> while ( my $line = <FILE> ) {
sw> chomp ($line);
sw> my @word = split / /, $line;
sw> my $count = 0;
sw> foreach my $aword ( @word ) {
sw> $aword =~ tr/^[\-a-zA-Z]//;
that isn't a good tr call/ it will not delete [] chars. those are not
metachars in tr. tr is NOT a regex!
sw> $aword =~ s/\'/\\\'/g;
sw> # $data->{ $aword } = 0 unless defined( $data->{ $aword } );
sw> $data->{ $aword }->{ $line } = 'line';
sw> $count++;
why do you need to do a manual count? you don't skip any words. just do
this outside the loop:
my $count = @word ;
sw> }
sw> }
sw> for my $word ( keys %{ $data } ) {
sw> my ( $imo, $owner, $manown, $manager );
sw> my $select = qq/SELECT $searcher /;
sw> my $from = qq/FROM owner, spd /;
sw> my $where = qq/WHERE MATCH( $searcher )
AGAINST('+$word'
sw> IN BOOLEAN MODE) /;
sw> my $join = qq/AND owner.num = spd.num/;
why do you have vars for each part? makes no sense or helps out. use a
here doc and make one longer sql string you can easily read:
my $select = <<SQL ;
SELECT $searcher FROM owner, spd
WHERE MATCH( $searcher ) AGAINST('+$word' IN BOOLEAN MODE)
AND owner.num = spd.num
SQL
nice and formatted and easy to edit and read.
sw> my $sth = $dbh->prepare( $query );
sw> $sth->execute;
if you use placeholders you would call execute with the args. then you
can use the sth again in a loop or wherever.
sw> for my $word ( keys %{ $data } ) {
sw> while( my ($field, $type) = each %{ $data->{ $word } } ) {
you need to pick better names than word and data. they are so generic i
can't follow the logic here.
and why did you quote an entire email and top post as well. learn to
bottom post and edit the quoted email.
uri
--
Uri Guttman ------ [email protected] -------- http://www.sysarch.com --
----- Perl Code Review , Architecture, Development, Training, Support ------
--------- Gourmet Hot Cocoa Mix ---- http://bestfriendscocoa.com ---------
--
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
http://learn.perl.org/
