in reply to Code review and question for parts script
Second, your kludge on 419 ... trying to encapsulate your database queries into one 'catch-all' subroutine is most likely going to lead to one nasty chunk of code. If you really want to abstract your database layer, try something like Class::DBI. One serious problem you have with that subroutine (execute_mysql_query) is that you connect to the database and disconnect from it each time you call that sub. Better is to connect once at the beginning of the script and store the database handle as a global variable. That subroutine really gains you very little right now, and seems rather hackish.
Third, i would just make $parts_algo a global variable. I like to declare vars that hold the command line arguments with use vars. For a small utility script like this, having your database handle and your options global shouldn't cause any problems ... unless you plan on using this under Apache::Registry (and i should hope not!). As for eval ... i personally don't like to eval just anything that the user can hand me. You should at least verify that the eval comforms to some kind of specification. Also, check the truth value of $@ to see if the eval fails.
Everything else seems fine ... oh yeah! Don't forget to remove you password when you post code online! ;)
Updated node: added a few things.
jeffa
L-LL-L--L-LL-L--L-LL-L-- -R--R-RR-R--R-RR-R--R-RR B--B--B--B--B--B--B--B-- H---H---H---H---H---H--- (the triplet paradiddle with high-hat)
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: (jeffa) Re: Code review and question for parts script
by bprew (Monk) on Oct 31, 2002 at 04:52 UTC | |
by jeffa (Bishop) on Oct 31, 2002 at 05:29 UTC |