Resume code of a SQL selecting function in Java

Question

I am new to Java so I started to learn by doing. Right now I am developing a store management software. I'm trying to do it the right and clean way, so I'm looking for some reviews of my code. The code is trying to resume the connection and the selecting from a database, so you will found a demonstration in the main method:

public class DBObject {
    Connection c=null;
    Statement stmt = null;
    ResultSet rs=null;

    public DBObject(Connection c,Statement stmt,ResultSet rs ){
        this.c=c;
        this.stmt=stmt;
        this.rs=rs;     
    }
    public DBObject(){

    }
    //excute select query (don't forget to close all of ResultSet,Statment,Connection objects)  
    public static DBObject executeSelectQuery(String query,String DBname){
        Connection c = null;
        Statement stmt = null;
        DBObject dbo=null;
        try {
              Class.forName("org.sqlite.JDBC");
              c = DriverManager.getConnection("jdbc:sqlite:"+DBname);
              c.setAutoCommit(false);
              System.out.println("Opened database successfully");
              stmt = c.createStatement();
              ResultSet rs = stmt.executeQuery(query);  
              dbo=new DBObject(c,stmt,rs);
            } catch ( Exception e ) {
              System.err.println( e.getClass().getName() + ": " + e.getMessage() );
              System.exit(0);
            }
        return dbo;
    }
    public static void main(String[] args) throws SQLException{
        String q="select *from compte";
        DBObject dbo=null;
        String id=null,password=null,nom=null,prenom=null,adresse=null,numtelephone=null;
        dbo=executeSelectQuery(q, "lib.db");
        while(dbo.rs.next()){
            id=dbo.rs.getString("compte_id");
            password=dbo.rs.getString("password");
            nom=dbo.rs.getString("nom");
            prenom=dbo.rs.getString("prenom");
            adresse=dbo.rs.getString("adresse");
            numtelephone=dbo.rs.getString("numtelephone");          
        }
        System.out.println(id+" "+password+" "+nom+" "+" "+prenom+" "+adresse+" "+numtelephone);
    }
}

Answer 1

Pool your connections

To set up a DB connection is usually very expensive, relative to the other tasks your application does:
The steps which are necessary to get a connection go right through:

• your application
• the JDBC Driver
• your application's JVM
• the network stack on the application machine
• the network
• the network stack on the DB machine
• the DB's authentication mechanism, checking roles and grants etc.

.. the acquired connection is then passed back through the same channels

If you do this for every Statement you would like to run on the DB, your application will spend most of its time waiting for connections to be established (and torn down after the Statement).

I would strongly suggest to read up on JDBC connection pooling. Which pooling library you would like to use depends on your project setup: Some Application Servers have a JDBC Connection Pool bundled with them.

Two stand alone JDBC connection pools are C3P0 and apache common's DBCP:
Example with C3P0
Example with DBCP

---

But more importantly:

Try not to re-invent the wheel

Your code is fine as a proof-of-concept / a worked out example of how a JDBC connection works. (Yes, it can be improved, as jrtapsell and J_H show, but as a proof-of-concept it's OK).

The underlying question is more interesting:
Should you write something yourself, which has many excellent solutions already?

Let's have a look at what you do:

• You establish a DB Connection (which should be pooled, see above):

  Connection c = DriverManager.getConnection("jdbc:sqlite:"+DBname);

• You use this DB Connection to run a query against the DB

  ResultSet rs = c.createStatement().executeQuery(query);

• You create a Java Object / Java Objects from the results of the queries (in your main method)

   while(dbo.rs.next()){
     id=dbo.rs.getString("compte_id");
     password=dbo.rs.getString("password");
     ..
     numtelephone=dbo.rs.getString("numtelephone");
   }

These are requirements of almost all applications, which have a multitude of well established, stable and convenient solutions.

I suggest, you read up on O/R Mapping (Wikipedia, hibernate) and a couple of its implementations (e.g. Hibernate getting started, JPA, Spring Data)

Of course, which tools and libraries you end up using in your application is up to you, but to my experience, it's better to have someone else make and fix an error than to have to make all errors yourself :-)

Answer 2

Class.forName

This is not necessary, and just risks causing issues. My previous answer about this lists the problems caused

Try-with-resouces

c = DriverManager.getConnection("jdbc:sqlite:"+DBname);

This could be in a try-with-resources block, so that the closing of the connection is closed no matter what happens, this also applies to the ResultSet and the Statement, and on some DBs this will create issues that will stop the system after a while.

Answer 3

Connection c=null;
Statement stmt = null;

Please be consistent. It is customary to put a space on each side of an = assignment and a space after each comma.

public DBObject() {
}

I think you meant to declare that private, to make the default constructor inaccessible. But it hardly matters, I would just delete it. Oh, wait, you don't use either constructor, nor the three attributes. Delete everything up to this point.

Your comment was very explicit about "don't forget to close all of...", and then you closed nothing. The try / catch should be in main instead, and it should be try / catch / finally, or better use try-with-resources.

Do not assign NULL when declaring variables, as that is default.

        } catch ( Exception e ) {

Do not be lazy - list the particular exceptions. Consider listing them in the function's signature in a throws clause, or alternatively replace the print & exit with a rethrow: throw new RuntimeException(e);

    String q="select *from compte";

It is usual to put a blank after the star. Just as this combines declaration with assignment, you might have chosen to declare and assign dbo in a single line.

Spell out each column name, rather than relying on * which supplies an arbitrary order. Databases and programs drift in different directions more often than you might think. Bit rot happens.

There's nothing wrong with declaring half a dozen column variables. But here, you might have simply asked println to call getString directly.

There's nothing wrong with string catenation, but here .format() would have been a good fit.