CODEWITHSUNDEEP
javadatabasejsonconnection
JS11
JS11

Reputation: 191

Is this a good way to execute query in java?

I am executing a query and putting result into JSONObject to return it to EXTJS page. Code works but I am not sure if this is the best or most efficient way to do it. I'll post my code please see if I need to improve it and where. I am fresh programmer so excuse obvious mistakes. Thanks in advance.

public JSONObject execQuery(String invoice, String id){

    StringBuffer sb = new StringBuffer();
    JSONObject json = new JSONObject();
    JSONObject data = new JSONObject();
    JSONArray jsArray = new JSONArray();

    try{
        // get conn
        conn = DBConnect.getInstance().dbOracleConnect();  

        // create query
        sb = new StringBuffer("SELECT * FROM table ");
        sb.append("WHERE rtrim(invoice) = ? AND ");
        sb.append("id = ? ");

        ps = conn.prepareStatement(sb.toString());
        ps.setString(1, invoice); 
        ps.setString(2, id); 

        rs = ps.executeQuery();

        while(rs.next()){
            json = new JSONObject();

            json.put("invoice", rs.getString("invoice"));
            json.put("id", rs.getString("id"));
            json.put("name", rs.getString("name"));
            json.put("gender", rs.getString("gender"));

            jsArray.put(json);
            // out put will be like [{"invoice":"111", "id":"123", "name":"sam", "gender":"male"}, {...}]               
        }
        data.put("data", jsArray);
        // out put will be like {"data":[{"invoice":"111", "id":"123", "name":"sam", "gender":"male"}, {...}]}
    }
    catch(Exception e){
        System.out.println("Error: " + e.toString());
    }
    finally {
        JDBCHelper.close(rs);
        JDBCHelper.close(ps);
        JDBCHelper.close(conn);
    }

    return data;
}

Upvotes: 1

Views: 1020

Answers (2)

abbas
abbas

Reputation: 7081

A few things you need to consider in your code:

  1. You are closing the connection through JDBCHelper, that means there should be a method in JDBCHelper to abstract away the details of getting a connection.

  2. Since you are not creating the query dynamically, you don't need to use StringBuffer/StringBuilder. A regular String is efficient for your case.

  3. The sb and json variables are initialized two times, once at the top and then again in the try block. Just declare these variables at the top and initialize them where they are used.

  4. You should bring down the initialization of json and jsArray right before the while loop and initialization of data after the loop.

Upvotes: 2

neurite
neurite

Reputation: 2824

Without the context, looks fine to me based on two assumptions:

1) The connection is obtained from a pool of connections. The use pattern seems to indicate that is the case. But you need to read the documentation of DBConnect and verify that.

2) The query is parameterized. You do NOT want the value to be concatenated directly into the SQL. That is inefficient and insecure. Your query is set up parameterized.

Some minor comments:

1) It looks like 'conn', 'rs', 'ps' are all fields of the class. I don't see the need. If you set them up as local variable, you class will become stateless - easier to read and maintain. Even better to declare the variables where they are used as suggested by others.

2) StringBuffer (or StringBuilder) here is overkill.

Upvotes: 1

Related Questions