• Post Reply Bookmark Topic Watch Topic
  • New Topic

Doubt with Design  RSS feed

 
Vineeth Menon
Ranch Hand
Posts: 79
Eclipse IDE Java Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Howdy Folks,

I have written a database connectivity class. This class is being called from actionPerformed which is in another class. Is this a good or a bad design? If it is a bad design how can I fix it?

 
Paul Clapham
Sheriff
Posts: 22841
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
1. It's possible for _resultSet to be null at line 31, because exceptions in the earlier try-block are handled and then you carry on as if they hadn't happened.

2. Why are con and statement class-level variables instead of local variables in the readDatabase() method?

3. You aren't closing your JDBC resources properly.

4. If you're using this class from a GUI via an actionPerformed method then writing data to the console is pointless.

5. There's no need to create a new instance of your JDBC driver class.

6. If you aren't going to do anything about the exception, at least output the stack trace so the programmer who supports it will be able to deal with them. Generally "e.printStackTrace()" is good enough.
 
Vineeth Menon
Ranch Hand
Posts: 79
Eclipse IDE Java Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,

This is not the actual code for production, I am in the process of perfecting it, that is why for the time being I am giving a System.out.println to print out test data. But I have made the changes as you have suggested. Is this fine? And is it a good practice to call this database class from another class on actionPerformed?

 
Tony Docherty
Bartender
Posts: 3271
82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As well as Paul's excellent observations:
1. Personally speaking I don't like the use of underscores in variable and method names and especially as the leading character of a public method name.
2. Also I don't like parameter names with the same name as instance variable names. In my experience, with beginners in particular, it invariably leads to mistakes.
3. It seems a little odd that _databaseConnection() calls readDatabase(). It's even more odd that readDatabase() doesn't return anything.
4. I would also suggest you use more descriptive method names for example readDatabase is a very generalised name but the method itself is hard coded to do a specific task.
 
Paul Clapham
Sheriff
Posts: 22841
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Vineeth Menon wrote:Hi,

This is not the actual code for production, I am in the process of perfecting it, that is why for the time being I am giving a System.out.println to print out test data.


But all we can see of the design is the code which you wrote to implement it. Apparently the design doesn't include actually using any of the data from that database, which is kind of pointless. Or if it does, you should have shown us the code which implements the real design.
 
Consider Paul's rocket mass heater.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!