> -----Original Message-----
> From: A mailing list for discussion about Sun Microsystem's Java Servlet
> API Technology. [mailto:[EMAIL PROTECTED]]On Behalf Of Steve
> Geach
> Sent: Monday, November 01, 1999 3:59 PM
> To: [EMAIL PROTECTED]
> Subject: Is this safe (corrected snippet)
>
>
> Sorry guys, missed the statement line out in my eagerness to cut the
> code down (thanks Tim). Corrected snippet below.
>
Steve-
Comments:
> If I went to a connection pool, had all my variables inside the doGet
> would this be thread safe ? If so then that is what I need, I dont need
> session state etc..
Yes, it would be thread safe if you had all your variables (including the
connection pool one) as local variables in doGet. But it wouldn't be much of a
connection pool then would it? :-) (since it would need to be recreated for
every request.)
To do the connection pool, you can:
(a) Make it an instance field for RepServlet servlets (similar to the Connection
object you have now).
Since a servlet engine is allowed to instantiate more than one servlet for each
class, you could end up having multiple connection pools lying around. Easy but
not recommended.
(b) Make it a static field (of class RepServlet, or of some other easily
reachable class).
As easy as (a), but has its objections (not OO design, not distributable).
(c) Make it an object of ServletContext.
A little more work but classiest (though involves downcasting).
This is a pretty popular discussion on this list. Check the archives for more
detail about implementation/design issues (specifically Craig McLanahan's
messages re design).
Also check the pool for null before you ask it for a connection; depending on
the implementation (some are provided with the engine), the pool object may have
gone stale.
> as user validation is carried out against the
> database at each request of a report.
If you encounter performance problems, this may be an area for improvement (must
you validate the user every single page request? isn't once per "use" enough? or
do their permissions change so drastically? can you get the web server to check
for permissions?). By the way, you can use the same connection pool for
validating users.
> private String SQLQuery1 = "select * from aTable a" +
> "where a.prod_no = ";
This ain't correct SQL (need space between 'a' and 'where'). Also it may as well
be "static" and "final". (Static, because it's not different from one servlet
instance to the next; final, because I don't think you want to change it during
runtime.)
> private String SQLQuery2 = "with ur";
I can't remember exactly what WITH does, so you'll have to check an SQL
reference. At the very least put a space in front of it or it will get stuck to
the product number when you concatenate. Also static + final.
>
> getQueryString ( req ); // method in servlet not shown, gets product
> validateUser ( id ) // validation of user method, not shown
>
> Statement stmt = con.createStatement ();
>
> if ( stmt.execute ( ( SQLQuery1 + product + SQLQuery2 ) ) )
Don't know where 'product' gets set but if it's subversively in 'getQueryString'
then you've got problems. Between that line and your concatenation, all sorts of
ghoulish things could have happened to it (specifically, other doGets could have
modified it). The suggestion is to do as Tim Panton advised, make 'product' a
local variable to whatever function actually needs it (for example,
getQueryString could just return the correct SQL query without saving the actual
int value anywhere).
If you think about it, it only makes sense. The servlet application doesn't have
or own a specific 'product', only the request does. So only the request or its
processor (that is, doGet, doPut or their delgates) should bother with a product
instance at all.
I just realized there's a naming confusion: I thought the "QueryString" in
getQueryString referred to an SQL query, but I think you meant to parse GET's
QUERY_STRING for variables. For one thing you pretty much never need to do the
parsing yourself (just use the req.getParameterValues()), for another, as
mentioned above, you shouldn't associate specific HTTP query parameters with a
servlet.
> ResultSetMetaData rsmd = rs.getMetaData ();
You only need this if you don't know the names/types of the columns in your
table, right?
> else
> {
> buf.append ( "<B>Hmmmm......</B> " );
> }
Just a personal quibble: don't put debugging code into your real code (doing
something appropriate when the select statement fails is part of the program
design, isn't it?). It makes it harder to excavate later. Use a debugger or a
test scaffolding.
-s
___________________________________________________________________________
To unsubscribe, send email to [EMAIL PROTECTED] and include in the body
of the message "signoff SERVLET-INTEREST".
Archives: http://archives.java.sun.com/archives/servlet-interest.html
Resources: http://java.sun.com/products/servlet/external-resources.html
LISTSERV Help: http://www.lsoft.com/manuals/user/user.html
