«

Hello all,

Im looking for an opinion regarding best practices. Recently a friend
and I were talking about how we write our code for our web
applications. I tend to keep my jdbc code with my logic in any
servlet I am writing. My friend says that this is bad practice and
that data quries should be broken out into data access objects with
methods that pass back a result set. Is he right? Is this really bad
practice or is it really just a different type of design pattern?
Love to hear everyones thoughts...


Im always doing something like this....

try {
Connection Conn = DriverManager.getConnection("jdbc:mysql://
sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");

// Do something with the Connection
Statement Stmt = Conn.createStatement();
ResultSet RS = Stmt.executeQuery("SELECT * from SOMETABLE");

while (RS.next()) {
out.println(RS.getString(1));
}
// Clean up
RS.close();
Stmt.close();
Conn.close();

}
catch (SQLException E) {
}

On Feb 7, 2:27 pm, "javerra" <tnava...@flipforwardinteractive.com>
wrote:
> Hello all,
>
> Im looking for an opinion regarding best practices. Recently a friend
> and I were talking about how we write our code for our web
> applications. I tend to keep my jdbc code with my logic in any
> servlet I am writing. My friend says that this is bad practice and
> that data quries should be broken out into data access objects with
> methods that pass back a result set. Is he right? Is this really bad
> practice or is it really just a different type of design pattern?
> Love to hear everyones thoughts...
>
> Im always doing something like this....
>
> try {
> Connection Conn = DriverManager.getConnection("jdbc:mysql://
> sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
>
> // Do something with the Connection
> Statement Stmt = Conn.createStatement();
> ResultSet RS = Stmt.executeQuery("SELECT * from SOMETABLE");
>
> while (RS.next()) {
> out.println(RS.getString(1));
> }
> // Clean up
> RS.close();
> Stmt.close();
> Conn.close();
>
> }
> catch (SQLException E) {
>
> }

Hello Javera,

You should have a look at MVC model.

And for the code you've written I would have wrote it this way :
Connection Conn = null;
Statement Stmt = null;
ResultSet RS = null;
try {
Conn =
DriverManager.getConnection("jdbc:mysql://
sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");

// Do something with the Connection
Stmt = Conn.createStatement();
RS = Stmt.executeQuery("SELECT * from SOMETABLE");

while (RS.next()) {
out.println(RS.getString(1));
}

}
catch (SQLException E) {
// handle the exception
}
finally{
// Clean up
try{
RS.close();
}
catch(Exception ex){}
RS = null;
try{
RS.close();
}
catch(Exception ex){}
RS = null;
try{
Conn.close();
}
catch(Exception ex){}
Conn= null;

}
because if you have an exception while executing the query
with your source code you'll never close your connection.

I hope this helps
DNass

I believe I have been implement an MVC design in the apps I've been
writing. My understanding is that with MVC I would have a "model" of
my data (just a plain javabean), Im using JDBC in a servlet to fill
my bean and do whatever processign logic needs done, this being the
"controller". Im then, stuffing that into a request or session
attribute and sending to a jsp "view". Is my understaning correct? I
guess this still leaves me confused about whether or not I should be
handling my data access form the servlet the way I have been. Thank
you for your thoughts!


On Feb 7, 9:47 am, "dnass" <dnasm...@gmail.com> wrote:
> On Feb 7, 2:27 pm, "javerra" <tnava...@flipforwardinteractive.com>
> wrote:
>
>
>
> > Hello all,
>
> > Im looking for an opinion regarding best practices. Recently a friend
> > and I were talking about how we write our code for our web
> > applications. I tend to keep my jdbc code with my logic in any
> > servlet I am writing. My friend says that this is bad practice and
> > that data quries should be broken out into data access objects with
> > methods that pass back a result set. Is he right? Is this really bad
> > practice or is it really just a different type of design pattern?
> > Love to hear everyones thoughts...
>
> > Im always doing something like this....
>
> > try {
> > Connection Conn = DriverManager.getConnection("jdbc:mysql://
> > sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
>
> > // Do something with the Connection
> > Statement Stmt = Conn.createStatement();
> > ResultSet RS = Stmt.executeQuery("SELECT * from SOMETABLE");
>
> > while (RS.next()) {
> > out.println(RS.getString(1));
> > }
> > // Clean up
> > RS.close();
> > Stmt.close();
> > Conn.close();
>
> > }
> > catch (SQLException E) {
>
> > }
>
> Hello Javera,
>
> You should have a look at MVC model.
>
> And for the code you've written I would have wrote it this way :
> Connection Conn = null;
> Statement Stmt = null;
> ResultSet RS = null;
> try {
> Conn =
> DriverManager.getConnection("jdbc:mysql://
> sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
>
> // Do something with the Connection
> Stmt = Conn.createStatement();
> RS = Stmt.executeQuery("SELECT * from SOMETABLE");
>
> while (RS.next()) {
> out.println(RS.getString(1));
> }
>
> }
> catch (SQLException E) {
> // handle the exception
> }
> finally{
> // Clean up
> try{
> RS.close();
> }
> catch(Exception ex){}
> RS = null;
> try{
> RS.close();
> }
> catch(Exception ex){}
> RS = null;
> try{
> Conn.close();
> }
> catch(Exception ex){}
> Conn= null;
>
> }
> because if you have an exception while executing the query
> with your source code you'll never close your connection.
>
> I hope this helps
> DNass

> catch(Exception ex){}

Don't ever do this. Failing silently might be the hardest bug you
will ever need to find.

Specifically in this case, if an exception is thrown trying to close
the resultset a cursor will be left open in the database. After
enough exceptions have been ignored you will reach the cursor limit of
your database and all of a sudden everything will start failing.

Do something like this instead:

doSomething() throws Exception {
try{
} finally {
try{
//deallocate resource A
} finally {
//deallocate resource B
}
}
}

On Feb 7, 4:28 pm, grasp06...@yahoo.com wrote:
> > catch(Exception ex){}
>
> Don't ever do this. Failing silently might be the hardest bug you
> will ever need to find.
>
> Specifically in this case, if an exception is thrown trying to close
> the resultset a cursor will be left open in the database. After
> enough exceptions have been ignored you will reach the cursor limit of
> your database and all of a sudden everything will start failing.
>
> Do something like this instead:
>
> doSomething() throws Exception {
> try{
> } finally {
> try{
> //deallocate resource A
> } finally {
> //deallocate resource B
> }
> }
>
> }

I meant
try{
// do something
catch (SQLException E) {
// handle the exception
}
finally{
// Clean up
try{
RS.close();
}
catch(Exception ex){}
RS = null;
try{
Stmt.close();
}
catch(Exception ex){}
Stmt= null;
try{
Conn.close();
}
catch(Exception ex){}
Conn= null;

}
once you close your resultset statement and connection I don't know
what else one can do ?

Thank you for all your thoughts on the proper exception handling and
clean up. The piece of code i posted was really just to illustrate my
original question and not meant to be any type of working example. I
apologize for not specifying that. That aside, any thoughts on my
original query?


On Feb 7, 10:53 am, "dnass" <dnasm...@gmail.com> wrote:
> On Feb 7, 4:28 pm, grasp06...@yahoo.com wrote:
>
>
>
> > > catch(Exception ex){}
>
> > Don't ever do this. Failing silently might be the hardest bug you
> > will ever need to find.
>
> > Specifically in this case, if an exception is thrown trying to close
> > the resultset a cursor will be left open in the database. After
> > enough exceptions have been ignored you will reach the cursor limit of
> > your database and all of a sudden everything will start failing.
>
> > Do something like this instead:
>
> > doSomething() throws Exception {
> > try{
> > } finally {
> > try{
> > //deallocate resource A
> > } finally {
> > //deallocate resource B
> > }
> > }
>
> > }
>
> I meant
> try{
> // do something
> catch (SQLException E) {
> // handle the exception
> }
> finally{
> // Clean up
> try{
> RS.close();
> }
> catch(Exception ex){}
> RS = null;
> try{
> Stmt.close();
> }
> catch(Exception ex){}
> Stmt= null;
> try{
> Conn.close();
> }
> catch(Exception ex){}
> Conn= null;
>
> }
> once you close your resultset statement and connection I don't know
> what else one can do ?

Please do not top-post. (Order corrected.)

"javerra" wrote:
>>> Im looking for an opinion regarding best practices. Recently a friend
>>> and I were talking about how we write our code for our web
>>> applications. I tend to keep my jdbc code with my logic in any
>>> servlet I am writing. My friend says that this is bad practice and
>>> that data quries should be broken out into data access objects with
>>> methods that pass back a result set. Is he right? Is this really bad
>>> practice or is it really just a different type of design pattern?
>>> Love to hear everyones thoughts...
>>> Im always doing something like this....
>>> try {
>>> Connection Conn = DriverManager.getConnection("jdbc:mysql://
>>> sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
>>> // Do something with the Connection
>>> Statement Stmt = Conn.createStatement();
>>> ResultSet RS = Stmt.executeQuery("SELECT * from SOMETABLE");
>>> while (RS.next()) {
>>> out.println(RS.getString(1));
>>> }
>>> // Clean up
>>> RS.close();
>>> Stmt.close();
>>> Conn.close();
>>> }
>>> catch (SQLException E) {
>>> }

There is an advantage and a disadvantage to the DAO approach. The advantage is
that it decouples data storage logic from business-rule logic. The
disadvantage is that it somewhat increases the up-front programming effort.

Decoupling data storage allows portability to other data storage schemes
(switching from JDBC to JNDI or a Web service, for example). It allows passing
of data entity value objects around without keeping a connection open. Passing
back non-Cached RowSets or ResultSets requires an active connection.

In a data-access-object (DAO) layer approach, the DAO objects accept and pass
back entities or collections, not ResultSets. There is no consciousness by the
clients of that layer that a Connection or a ResultSet or Statement exists.

dnass wrote:
>> You should have a look at MVC model.
>>
>> And for the code you've written I would have wrote it this way :
>> Connection Conn = null;
>> Statement Stmt = null;
>> ResultSet RS = null;

You dcn't actually need the extra initialization of these variables. Also, it
is conventional to name variables with a lower-case first letter, to
distinguish them from class identifiers.

>> try {
>> Conn =
>> DriverManager.getConnection("jdbc:mysql://
>> sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
>>
>> // Do something with the Connection
>> Stmt = Conn.createStatement();
>> RS = Stmt.executeQuery("SELECT * from SOMETABLE");
>>
>> while (RS.next()) {
>> out.println(RS.getString(1));
>> }
>>
>> }
>> catch (SQLException E) {
>> // handle the exception
>> }
>> finally{
>> // Clean up
>> try{
>> RS.close();
>> }
>> catch(Exception ex){}
In real life one would likely want to log this exception. Also, some suggest
that one should not catch "kitchen_sink" exceptions.

>> RS = null;
>> try{
>> RS.close();
This would throw a NullPointerException.
>> }
>> catch(Exception ex){}
>> RS = null;
>> try{
>> Conn.close();
>> }
>> catch(Exception ex){}
>> Conn= null;
>>
>> }
>> because if you have an exception while executing the query
>> with your source code you'll never close your connection.

javerra wrote:
> I believe I have been implement an MVC design in the apps I've been
> writing. My understanding is that with MVC I would have a "model" of
> my data (just a plain javabean),

It doesn't actually *have* to be a JavaBean, since model classes are primarily
behavioral and not really value objects. They likely will have some
attributes, though, so in that sense they are likely to follow bean accessor
patterns.

> Im using JDBC in a servlet

In the MVC pattern there would be no servlet directly accessing JDBC. That
would be done through a model class or a DAO class.


> to fill my bean and do whatever processign logic needs done, this being the
> "controller".

The controller is supposed to handle only parsing a request, its dispatch to
model logic, then navigation to the subsequent view. All other logic happens
in the model.

> Im then, stuffing that into a request or session
> attribute and sending to a jsp "view".

Stuffing what "that"? The logic object? The logic object is primarily
behavioral and should not reside as a session or request attribute. It may,
however, determine via business rules that some value object or collection
thereof should be "stuffing" for the request or session (or wherever).

> I guess this still leaves me confused about whether or not I should be
> handling my data access form the servlet the way I have been.

The servlet really should not be the one to talk to the data store directly,
in nearly all these architectures. That is a job left to the model.

- Lew

On Feb 7, 2:00 pm, Lew <l...@nospam.lewscanon.com> wrote:
> Please do not top-post. (Order corrected.)
>
>
>
> "javerra" wrote:
> >>> Im looking for an opinion regarding best practices. Recently a friend
> >>> and I were talking about how we write our code for our web
> >>> applications. I tend to keep my jdbc code with my logic in any
> >>> servlet I am writing. My friend says that this is bad practice and
> >>> that data quries should be broken out into data access objects with
> >>> methods that pass back a result set. Is he right? Is this really bad
> >>> practice or is it really just a different type of design pattern?
> >>> Love to hear everyones thoughts...
> >>> Im always doing something like this....
> >>> try {
> >>> Connection Conn = DriverManager.getConnection("jdbc:mysql://
> >>> sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
> >>> // Do something with the Connection
> >>> Statement Stmt = Conn.createStatement();
> >>> ResultSet RS = Stmt.executeQuery("SELECT * from SOMETABLE");
> >>> while (RS.next()) {
> >>> out.println(RS.getString(1));
> >>> }
> >>> // Clean up
> >>> RS.close();
> >>> Stmt.close();
> >>> Conn.close();
> >>> }
> >>> catch (SQLException E) {
> >>> }
>
> There is an advantage and a disadvantage to the DAO approach. The advantage is
> that it decouples data storage logic from business-rule logic. The
> disadvantage is that it somewhat increases the up-front programming effort.
>
> Decoupling data storage allows portability to other data storage schemes
> (switching from JDBC to JNDI or a Web service, for example). It allows passing
> of data entity value objects around without keeping a connection open. Passing
> back non-Cached RowSets or ResultSets requires an active connection.
>
> In a data-access-object (DAO) layer approach, the DAO objects accept and pass
> back entities or collections, not ResultSets. There is no consciousness by the
> clients of that layer that a Connection or a ResultSet or Statement exists.
>
> dnass wrote:
> >> You should have a look at MVC model.
>
> >> And for the code you've written I would have wrote it this way :
> >> Connection Conn = null;
> >> Statement Stmt = null;
> >> ResultSet RS = null;
>
> You dcn't actually need the extra initialization of these variables. Also, it
> is conventional to name variables with a lower-case first letter, to
> distinguish them from class identifiers.
>
>
>
> >> try {
> >> Conn =
> >> DriverManager.getConnection("jdbc:mysql://
> >> sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
>
> >> // Do something with the Connection
> >> Stmt = Conn.createStatement();
> >> RS = Stmt.executeQuery("SELECT * from SOMETABLE");
>
> >> while (RS.next()) {
> >> out.println(RS.getString(1));
> >> }
>
> >> }
> >> catch (SQLException E) {
> >> // handle the exception
> >> }
> >> finally{
> >> // Clean up
> >> try{
> >> RS.close();
> >> }
> >> catch(Exception ex){}
>
> In real life one would likely want to log this exception. Also, some suggest
> that one should not catch "kitchen_sink" exceptions.
>
> >> RS = null;
> >> try{
> >> RS.close();
>
> This would throw a NullPointerException.
>
>
>
> >> }
> >> catch(Exception ex){}
> >> RS = null;
> >> try{
> >> Conn.close();
> >> }
> >> catch(Exception ex){}
> >> Conn= null;
>
> >> }
> >> because if you have an exception while executing the query
> >> with your source code you'll never close your connection.
> javerra wrote:
> > I believe I have been implement an MVC design in the apps I've been
> > writing. My understanding is that with MVC I would have a "model" of
> > my data (just a plain javabean),
>
> It doesn't actually *have* to be a JavaBean, since model classes are primarily
> behavioral and not really value objects. They likely will have some
> attributes, though, so in that sense they are likely to follow bean accessor
> patterns.
>
> > Im using JDBC in a servlet
>
> In the MVC pattern there would be no servlet directly accessing JDBC. That
> would be done through a model class or a DAO class.
>
> > to fill my bean and do whatever processign logic needs done, this being the
> > "controller".
>
> The controller is supposed to handle only parsing a request, its dispatch to
> model logic, then navigation to the subsequent view. All other logic happens
> in the model.
>
> > Im then, stuffing that into a request or session
> > attribute and sending to a jsp "view".
>
> Stuffing what "that"? The logic object? The logic object is primarily
> behavioral and should not reside as a session or request attribute. It may,
> however, determine via business rules that some value object or collection
> thereof should be "stuffing" for the request or session (or wherever).
>
> > I guess this still leaves me confused about whether or not I should be
> > handling my data access form the servlet the way I have been.
>
> The servlet really should not be the one to talk to the data store directly,
> in nearly all these architectures. That is a job left to the model.
>
> - Lew


Thank you for the detailed response. Its a lot clearer now and I see
some coding habits I have will have to change.

javerra wrote:
> Im looking for an opinion regarding best practices. Recently a friend
> and I were talking about how we write our code for our web
> applications. I tend to keep my jdbc code with my logic in any
> servlet I am writing. My friend says that this is bad practice and
> that data quries should be broken out into data access objects with
> methods that pass back a result set. Is he right? Is this really bad
> practice or is it really just a different type of design pattern?
> Love to hear everyones thoughts...
>
> Im always doing something like this....
>
> try {
> Connection Conn = DriverManager.getConnection("jdbc:mysql://
> sql.useractive.com/USERNAME?user=USERNAME&password=PASSWORD");
>
> // Do something with the Connection
> Statement Stmt = Conn.createStatement();
> ResultSet RS = Stmt.executeQuery("SELECT * from SOMETABLE");
>
> while (RS.next()) {
> out.println(RS.getString(1));
> }
> // Clean up
> RS.close();
> Stmt.close();
> Conn.close();
>
> }
> catch (SQLException E) {
> }

I think there are a couple of problems with your approach:
* JDBC calls in your servlet (servlet is controller layer,
JDBC calls belong in data access layer)
* the usage of out.println (servlet is controller layer,
output generation belong in presentation layer)

So create a data access layer with some classes that
for data retrieval has methods to return a single object or
a collection of objects. Do not return a ResultSet, because
that is still tied to the implementation of the data access
layer.

Call that from your servlet, store it in request and forward
it to a JSP pages that displays the data. Displays the data
using a taglib not with scriptlet code.

Arne

Arne Vajhøj wrote:

> I think there are a couple of problems with your approach:
> * JDBC calls in your servlet (servlet is controller layer,
> JDBC calls belong in data access layer)
> * the usage of out.println (servlet is controller layer,
> output generation belong in presentation layer)

But you aren't addressing the question: you are /assuming/ that the
architecture you descibe represents best practice; the OP wants to know
/whether/ it is (always) best practise, and presumably why.

For what little it's worth: I have a distrust of the complexities implied by
the commonly advocated architecture, with all its layers and TLAs, and would
generally take the approach that if a simpler approach /does/ work (and can be
expected to continue to work as the application evolves) then that is to be
preferred.

-- chris