HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

DAO to create, update, and delete a project

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
updatecreatedeletedaoprojectand

Problem

I'm trying to write my first "complex" program in Java. So far, I created a local H2 database but I'm not sure about the quality of my code (I'm newbie in Java) because I don't know how I can deal with exceptions and I don't feel comfortable with the JDBC code.

```
public boolean create(Project project) {
String id = UUID.randomUUID().toString().toUpperCase().replace("-", "");
project.setId(id);

try {
Class.forName("org.h2.Driver");
Connection connection = DriverManager.getConnection("jdbc:h2:~/dao_db", "sa", "");
PreparedStatement preparedStatement = connection.prepareStatement("INSERT INTO PROJECT VALUES(?, ?,?)");

preparedStatement.setString(1, id);
preparedStatement.setString(2, project.getName());
preparedStatement.setString(3, project.getTrigram());
int resultSet = preparedStatement.executeUpdate();

if(resultSet!=0)
{
System.out.println("Project created");
connection.close();
return true;
}

else {
System.out.println("No project was created ");
connection.close();
return false;
}

} catch (ClassNotFoundException | SQLException e) {
e.printStackTrace();
}
return false;

}

@Override
public boolean update(Project project){

try {
Class.forName("org.h2.Driver");

Connection connection = DriverManager.getConnection("jdbc:h2:~/dao_db", "sa", "");
PreparedStatement preparedStatement = connection.prepareStatement("UPDATE PROJECT SET NAME=?, TRIGRAM = ? WHERE ID=?");

preparedStatement.setString(1, project.getName());
preparedStatement.setString(2, project.getTrigram());
preparedStatement.setString(3, project.getId());
int resultSet = preparedStatement.executeUpdate();

if(resultSet==0) {
System.out.println("The project was not updated ");
connection.close();
return fal

Solution

Nitpick

public boolean create(Project project)  {
 String id = UUID.randomUUID().toString().toUpperCase().replace("-", "");
 project.setId(id);


The last two lines should be indented relative to the method declaration. Perhaps this was just a copy/paste error that doesn't exist in the original code.

try

try {
        Class.forName("org.h2.Driver");
        Connection connection = DriverManager.getConnection("jdbc:h2:~/dao_db", "sa", "");
        PreparedStatement preparedStatement = connection.prepareStatement("INSERT INTO PROJECT VALUES(?, ?,?)");

        preparedStatement.setString(1, id);
        preparedStatement.setString(2, project.getName());
        preparedStatement.setString(3, project.getTrigram());
        int resultSet = preparedStatement.executeUpdate();

        if(resultSet!=0)
        {
            System.out.println("Project created");
            connection.close();
            return true;
        }

        else {
            System.out.println("No project was  created ");
            connection.close();
            return false;
        }

    } catch (ClassNotFoundException | SQLException e) {
        e.printStackTrace();
    }


This is a really long try block, and most of the code doesn't seem to need it. Also, it doesn't make use of the try with resources, even though it seems to exactly cover the use case. Instead consider breaking it into two smaller try blocks.

try {
        Class.forName("org.h2.Driver");
    } catch (ClassNotFoundException e) {
        e.printStackTrace();
        return false;
    }


All this code does is verify that the class exists. I would actually do this outside a try block, as it seems like a fatal error. But this version at least isolates the class problem from the SQLException issue, as they have different responses.

int resultSet = 0;


Technically, the 0 is unnecessary here, as it is already the default. I find it clearer to initialize it explicitly. Others disagree.

Removing from the try block will allow it to be used outside the block later.

try (Connection connection = DriverManager.getConnection("jdbc:h2:~/dao_db", "sa", "")) {
        PreparedStatement preparedStatement = connection.prepareStatement("INSERT INTO PROJECT VALUES(?, ?,?)");

        preparedStatement.setString(1, id);
        preparedStatement.setString(2, project.getName());
        preparedStatement.setString(3, project.getTrigram());

        resultSet = preparedStatement.executeUpdate();
    } catch (SQLException e) {
        e.printStackTrace();
    }


Now you don't have to worry about explicitly closing the connection. The try with resources will handle that automatically no matter how or when you leave the try block.

Note that in the original code, an exception after the connection was made but before the connection was explicitly closed would bypass closing the connection. You could have used a finally block to work around this, but the try with resources is simpler and more reliable.

if (resultSet != 0) {
        System.out.println("Project created");
        return true;
    }

    System.out.println("No project was created");
    return false;


This block of code cannot generate either of the caught exceptions. As such, it does not need to be in the try block. So go ahead and take it out.

Consistent formatting

There are three common ways to handle a {} block. You use all three. Please pick one and stick to it consistently. Consistent formatting makes it easier to both read and modify the code.

Start and end on the same line:

} else {


Start and end on separate lines:

}
    else
    {


Closing } alone:

}
    else {


Also, note that the second and third versions allow you to put extra blank lines between the braces and the else. Please don't do that. When you do that, it obscures that the previous block has an else. It's much clearer if you treat the braces and the else as one symbol and always write them the same way. Then the reader can immediately see which block ends and which will continue later.

Personally, I prefer the first form, but all three are in common use. The Java coding standard also prefers the first form, but some prefer one of the others. Consistent use of any of them within your code will allow most programmers to adapt.

Code Snippets

public boolean create(Project project)  {
 String id = UUID.randomUUID().toString().toUpperCase().replace("-", "");
 project.setId(id);
try {
        Class.forName("org.h2.Driver");
        Connection connection = DriverManager.getConnection("jdbc:h2:~/dao_db", "sa", "");
        PreparedStatement preparedStatement = connection.prepareStatement("INSERT INTO PROJECT VALUES(?, ?,?)");

        preparedStatement.setString(1, id);
        preparedStatement.setString(2, project.getName());
        preparedStatement.setString(3, project.getTrigram());
        int resultSet = preparedStatement.executeUpdate();

        if(resultSet!=0)
        {
            System.out.println("Project created");
            connection.close();
            return true;
        }

        else {
            System.out.println("No project was  created ");
            connection.close();
            return false;
        }

    } catch (ClassNotFoundException | SQLException e) {
        e.printStackTrace();
    }
try {
        Class.forName("org.h2.Driver");
    } catch (ClassNotFoundException e) {
        e.printStackTrace();
        return false;
    }
int resultSet = 0;
try (Connection connection = DriverManager.getConnection("jdbc:h2:~/dao_db", "sa", "")) {
        PreparedStatement preparedStatement = connection.prepareStatement("INSERT INTO PROJECT VALUES(?, ?,?)");

        preparedStatement.setString(1, id);
        preparedStatement.setString(2, project.getName());
        preparedStatement.setString(3, project.getTrigram());

        resultSet = preparedStatement.executeUpdate();
    } catch (SQLException e) {
        e.printStackTrace();
    }

Context

StackExchange Code Review Q#124276, answer score: 3

Revisions (0)

No revisions yet.