>>>>> "sw" == shawn wilson <ag4ve...@gmail.com> 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  ------  u...@stemsystems.com  --------  http://www.sysarch.com --
-----  Perl Code Review , Architecture, Development, Training, Support ------
---------  Gourmet Hot Cocoa Mix  ----  http://bestfriendscocoa.com ---------

-- 
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/


Reply via email to