I would put in the OS respose at the open file line with $!, even tho' it may not be appropriate considering the user of the script, if there are any errors they can give back the exact details under $!.
Using a rather personal style (if perf isnt an issue) I like to stick my SQL into an OS file that I suck in. I like to do this because I can edit the sql directly, meaning I cant accidently modify the supporting code.
Another personal thing, I would perhaps issuse a challenge for the database authen, rather than store the pwd. It doesnt take a rocket scientist for a novice to use google, learn a few perly bits, and get back the pwd they could use in another app. If you must store the password, why not use a two way encryption engine (such as DES or 3DES) and store the key in a protected file somewhere. Not really what I consider brutely secure, but better than effectively keeping it in the clear.
You dont do any decent checking of the header/booth vars. I am not familiar with MySQL (nor an expert on the SQL 92 std) , however i would think it would lend itself to the old subquery trick where the column to be inserted is the result of a (sub)query itself. (also if a user accidently enters a ' (single quote) it will bugger up the query (well, it will with postgres and oracle))
If $booth_table doesnt change thru'out the code, why not:
Personally, I like the constant, but it depends on your style.
OK, so i'm being incredilby nit-picky, and anal retentive about it all. Above all, it seems like a good bit of code, easy to read and easy to maintain. Most of my suggestions are personal preference and could be just plainly be avoided.
One thing I never avoid in my code that will be used by untrusted 3rd parties is logging. Grab an IP, or provide authen to use the script, but log every untrusted 3rd party that may use your script. It definately a CYA measure, but can solve disputes, uncover problems you may have with security/perf et al. IMHO very important.
Another thing I would not avoid is checking the return of any DBI execute statements and handling the errors accordingly (ie to fit your context).
Overall a good bit of code, perhaps an 7..8/10 by my standards, well done (++).
Update:Thanks to Zaxo who pointed out that raiseerror is being called. I did notice that, however I meant some explicit error handling (altho it may not be needed to this type of app)In reply to Re: For Review: DB Import
by Ryszard
in thread For Review: DB Import
by impossiblerobot
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |