aspose file tools*
The moose likes JDBC and the fly likes JDBC Code Review Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Java 8 in Action this week in the Java 8 forum!
JavaRanch » Java Forums » Databases » JDBC
Bookmark "JDBC Code Review" Watch "JDBC Code Review" New topic
Author

JDBC Code Review

Danish Shaukat
Ranch Hand

Joined: Nov 16, 1999
Posts: 340
Hi all,

I am currently working on a discussion forum application.

The method below is used to start a new thread. Multiple tables are being updated (inserts and updates). Some tables are being queried for data required for updates.

Transaction management is in place.

Apart from this I also have a custom exception which is thrown in case of an empty result set.

If any kind of exception is thrown, the transaction is rolled back. I am also rethrowing the exception to the calling program for some further processing.

Can any one go through the code and suggest how to improve it? There is always room for optimization and refactoring so all suggestions are welcomed.

Regards,
Danish


Sebastian Janisch
Ranch Hand

Joined: Feb 23, 2009
Posts: 1183
Your code looks decent, but it's a huge load of boilerplate. Why don't you check out JDBCSupport ? It can minimize the code for you ;-)


JDBCSupport - An easy to use, light-weight JDBC framework -
Danish Shaukat
Ranch Hand

Joined: Nov 16, 1999
Posts: 340
Sebastian Janisch wrote:Your code looks decent, but it's a huge load of boilerplate. Why don't you check out JDBCSupport ? It can minimize the code for you ;-)


Yes, I agree that the method is really long. Checked out JDBC Support. It is still in beta and was released recently.
Sebastian Janisch
Ranch Hand

Joined: Feb 23, 2009
Posts: 1183
Danish Shaukat wrote:
Yes, I agree that the method is really long. Checked out JDBC Support. It is still in beta and was released recently.


It's my project and it's quite young. Check it out and let me know how it's going ;-)
Wilson Cursino
Greenhorn

Joined: Dec 27, 2009
Posts: 6
Hi Danish,

Looks neat the code, however even for a basic code you should consider the JDBCSupport.

However, in MHO I think you should consider use hibernate, it will turn your life so easy and it isn't that hard.

Cheers and good luck!



Wilson Cursino
IBM IT Architect Certification [2007], IBM IT Architect Accreditation [2006], SCEA Aug 2004, SCWCD Jan 2004 92%, SCJP Jan 2003 93%, SCJA Jun 2005 98%
Jeanne Boyarsky
internet detective
Marshal

Joined: May 26, 2003
Posts: 29287
    
140

My comments:
1) I would use a ConnectionPool rather than obtaining the connection via username/password each time.
2) I agree on the boilerplate comment. Many people write their own superclass with the boilerplate or use an ORM framework like Hibernate.
3) startNewThread() is way too long. While the exact length of a method is debatable, most people agree is should fit on a screen or two. Look for snippets you can extract into new methods - like reading from the resultset.
4) I would list out the columns rather an "select *". In fact, I would also specifically list the columns I needed - in this case threadId. Returning columns you don't need as a performance impact.
5) threadRS.close() should be in a finally block.
6) Having to null out variables is an anti-pattern that signifies the code that uses those variables should have been in a separate method.
7) I'd name "selectThread" as "selectTheadStmt". The former sounds like it is of Thread type.
8) "// get Unix time stamp value in seconds " - These comments are also an anti-pattern that the code the comment applies to should be in method named getUnixTimeStampValueInSeconds() and the like.
9) Why do you clear the parameters when the prepared statement has just been created?


[Blog] [JavaRanch FAQ] [How To Ask Questions The Smart Way] [Book Promos]
Blogging on Certs: SCEA Part 1, Part 2 & 3, Core Spring 3, OCAJP, OCPJP beta, TOGAF part 1 and part 2
Jeanne Boyarsky
internet detective
Marshal

Joined: May 26, 2003
Posts: 29287
    
140

On the length thing, it isn't just boilerplate. It is that there is so much going on that it is hard for a reader to figure out what the code does. What if someone other than you has to maintain the code? For that matter, what if you have to in a year?

Compare the code in the first post to the following. Your method names would be clearer of course as they could describe what the queries do. They'd also be more real and pass the proper variables around.



In addition to fitting on a screen, my version has business meaning - you can tell why each code block is needed. It also provides chunks that are possible to reuse.

[edited to fix typo]
Danish Shaukat
Ranch Hand

Joined: Nov 16, 1999
Posts: 340
Hi Wilson and Jeanne,

Thanks for going through the lengthy code. Your comments are valuable. I will be changing the code as per guidelines.

Regards,
Danish
Wilson Cursino
Greenhorn

Joined: Dec 27, 2009
Posts: 6
Danish you are welcome. Let me know if you need any help with the conversion.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: JDBC Code Review
 
Similar Threads
Help debug this program
question on JDBC connection to MySQL
how to upload pdf files to My sql with java
The value for the useBean class attribute is invalid.
how to insert an image into MYSQL database using java