patternjavaMinor
Basic Java database
Viewed 0 times
javadatabasebasic
Problem
I just wrote my first Java database program for the purpose of getting feedback on the implementation and coding. It has 1 table and 2 buttons and prompts the user to select a folder, lists the contents of the folder in the table and lists the hash of the files in the table and writes it to a database.
It works fine, but I have no idea if I coded it cleanly and split the program into the proper packages and classes. It's definitely a beginner level project so there is nothing too complex about it. I'm also using the h2 database because I was told its the most efficient for small databases. Could you all please give me feedback on this?
I used NetBeans 7.3 and uploaded the full project here. This is the class I'm using for the database. It contains most of the code I have in question, but there are other parts of the project as a whole I'm concerned if they were organized correctly:
```
public class CDatabaseLayer {
private static ArrayList fileList = new ArrayList();
private static Server server;
private static JdbcDataSource ds = new JdbcDataSource();
private static Connection conn;
private static int lastId = 0;
private static Statement stat;
private static ResultSet rs;
private static String query;
static public ArrayList getFileList() {
connectDatabase();
return fileList;
}
static public boolean connectDatabase()
{
System.out.println("Attempting to connect to database.");
if(server == null) {
try {
server = Server.createTcpServer();
server = server.start();
} catch (SQLException ex) {
System.out.println("connectDatabase createTcpServer() exception: "+ex);
return false;
}
}
//return false if connected
if(conn != null) {
System.out.println("Already established DB connection.");
return false;
} else {
try {
It works fine, but I have no idea if I coded it cleanly and split the program into the proper packages and classes. It's definitely a beginner level project so there is nothing too complex about it. I'm also using the h2 database because I was told its the most efficient for small databases. Could you all please give me feedback on this?
I used NetBeans 7.3 and uploaded the full project here. This is the class I'm using for the database. It contains most of the code I have in question, but there are other parts of the project as a whole I'm concerned if they were organized correctly:
```
public class CDatabaseLayer {
private static ArrayList fileList = new ArrayList();
private static Server server;
private static JdbcDataSource ds = new JdbcDataSource();
private static Connection conn;
private static int lastId = 0;
private static Statement stat;
private static ResultSet rs;
private static String query;
static public ArrayList getFileList() {
connectDatabase();
return fileList;
}
static public boolean connectDatabase()
{
System.out.println("Attempting to connect to database.");
if(server == null) {
try {
server = Server.createTcpServer();
server = server.start();
} catch (SQLException ex) {
System.out.println("connectDatabase createTcpServer() exception: "+ex);
return false;
}
}
//return false if connected
if(conn != null) {
System.out.println("Already established DB connection.");
return false;
} else {
try {
Solution
This is what I noticed after taking a quick look:
- I'm not a big fan of the static/singleton-ness of this class. Those attributes make this class harder to mock and unit test. Take a look at POJO/dependency injection (you don't need a container to do DI).
- I would consider separating the database/Connection opening stuff into its own class. There should be separation of what the data access/service layer from the resource/connection code. Take a look at DAO/Service patterns.
- getFileList() returns a concrete List implementation instead of the List interface.
- A lot of your methods don't close the ResultSet and Statement objects once they are done with them. They should be closed in finally blocks.
- Your catch blocks aren't doing much besides printing to System.out. You need to either handle recoverable exceptions there or propogate them up the stack as a RuntimeException.
- Not sure why you need newStatement().
- You are manually creating and executing SQL strings. This is a bad idea. Replace these with PreparedStatements.
- This class isn't threadsafe. It should be documented as much.
Context
StackExchange Code Review Q#26242, answer score: 4
Revisions (0)
No revisions yet.