• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Need Code Review

 
Elton Hughes
Ranch Hand
Posts: 72
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello All and thank you for your time.
Background: I work for a small non-profit agency. My primary responsibility is developing ad hoc reoprts for management from our Informix database. My usual tools are SQL and Perl. In June, I found Head First Java in a local bookstore and loved it. I went through the whole book and felt comfortable with all that I learned.
Fast forward to today. Our Informix database is getting replaced by a MS SQL Server. When I asked the vendor about ad hoc reporting tools, I was told that none were in the original proposal, but you can use ODBC and/or JDBC to do what you need. <expletive deleted by author>
So, today I put together the following.

Crude, but it works.
But I am also certain that it can be written better, after all, I am a beginner at this. So JavaRanchers, what would you do to make it better.
Thanks!
Elton
 
Jeanne Boyarsky
author & internet detective
Marshal
Posts: 34198
340
Eclipse IDE Java VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Elton,
The code looks good and functional. It's not really necessary to put closing the statement/connection inside a try/catch. If the first one fails, the second will as well. It's easier to just let the outer try/catch deal with the exception.
 
Jamie Robertson
Ranch Hand
Posts: 1879
MySQL Database Suse
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Elton, as you have experienced, databases come and go. IP addresses change, ports change and the such. So it is a good idea to separate the connection creation from the programs using it so that you only have to maintain it in one place( not hard-coded in several programs ).
it can be as simple as:


you can modify it to be more flexible as you need it to be ( i.e. loading multiple database instances and drivers ) but all you really need to do now is to separate the connection logic from the programs that use it.
[ just edited to get rid of the extra try/catch blocks and to add a connection.close to your exception block to ensure db resources are released. ]
Jamie
[ December 17, 2003: Message edited by: Jamie Robertson ]
 
Wayne L Johnson
Ranch Hand
Posts: 399
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Why not use the "finally" block for cleaning up database resources? That's why it's there.

By putting the code in the "finally" block you are guaranteed that it will be executed regardless of whether you threw an exception or not. Also, with some [most? all?] databases it's a good idea to explicitly close the result set as well as the statement and database.
This is a good model to use when doing database access, and helps prevent dangling resources on the database.
[ December 18, 2003: Message edited by: Wayne L Johnson ]
 
Dana Hanna
Ranch Hand
Posts: 227
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As a Java enterprise developer of 5 years - the above is right on. Better safe than sorry when cleaning up. Too many connection leaks around my place in the past.
 
Elton Hughes
Ranch Hand
Posts: 72
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello All,
I am sorry for being so tardy with this. But I did modify my original program and it looks like this:

It looks much better, at least to my novice eye.
My thanks to you all.
Elton
 
Lu Battist
Ranch Hand
Posts: 104
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'd suggest that simply doing a tab delimited output while functional isn't pretty. If your datatypes are varchar then the tabs won't always line up and it can get hard to read. I fairly basic html page output could greatly impove the look. Put the results in a table complete with column headers. The table format will keep everything lined up. The column header can be done manually if you always do custom reports. If you want a dynamic sql query then you'll have to get and use resultSetMetaData.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic