Using database connection and GUI

Question

In my project there are a lot of GUI so I have a class for each GUI. I created a class to connect to my db (MySQL). Remember, I don't have problems with code not working. The code (important to this question) in the connectiondb class is:

public static void loadDriver_doConnection(){
    try {
        Class.forName(jdbc_driver);

        try { conn =  DriverManager.getConnection(url1,"root","pswww");
        }
        catch(SQLException es)
        { //es.printStackTrace();

        }

    }
    catch (ClassNotFoundException ex) { 
        //ex.printStackTrace();

    }
}

    public static Connection loadDriver_doConnection_backConn(){
        try {
            Class.forName(jdbc_driver);

            try { conn =  DriverManager.getConnection(url1,"root","pswww");

            }
            catch(SQLException es)
            { //es.printStackTrace();

            }

        }
        catch (ClassNotFoundException ex) { 
            //ex.printStackTrace();

        }
                return conn;
    }
    static ResultSet doQuery2(String sql, boolean full_navigable)
{
    if (conn != null)
    {            
        try // 
        {
            if (full_navigable)
                st = conn.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE);
            else
                st = conn.createStatement();

            rs = st.executeQuery(sql);
        }
        catch (SQLException ex) { 

            JOptionPane.showMessageDialog(null,ex.getMessage());
        }

        return rs;
    }
    else
    {

        return null;
    }

And now I'm showing you an example (with no try-catch to control input) of my code on a GUI where I do an insert/delete:

 String sqlinsart="insert into Art (CF,Name,Surname,Date,City born,City live,road,Num,Email,Tel,Cell) values(?,?,?,?,?,?,?,?,?,?,?)";
         Connection conn=Connectiondb.loadDriver_doConnection_backConn();
         PreparedStatement ps = conn.prepareCall(sqlinsart);
         ps.setString(1, txtformattedcf.getText().trim());
         ps.setString(2, txtname.getText().trim());
         ps.setString(3, txtsurname.getText().trim());
         SimpleDateFormat formatDate = new SimpleDateFormat("dd/MM/yyyy");
         java.util.Date invoiceDate = formatDate.parse(txtformatteddate.getText().trim());
         java.sql.Date sqlDate = new java.sql.Date(invoiceDate.getTime());
         ps.setDate(4, sqlDate);
         ps.setString(5, txtcityb.getText().trim());
         ps.setString(6, txtcity.getText().trim());
         ps.setString(7, txtroad.getText().trim());
         ps.setString(8, txtformattednum.getText().trim());
         ps.setString(11, txtemail.getText().trim());
         ps.setString(12, txtformattedtel.getText().trim());
         ps.setString(13, txtformattedcell.getText().trim());
         int n = ps.executeUpdate();
         if (n>0){
             JOptionPane.showMessageDialog(null, "Insert ok");

         }

DELETE (these actions are done by bottons):

 String userelimart=txtformattedcf.getText().trim();
         String sqlelimart="delete from Art where Art.CF='"+userelimart+"'";
         Connection conn=Connectiondb.loadDriver_doConnection_backConn();
         Statement sent=conn.createStatement();
         int n =sent.executeUpdate(sqlelimart);
         if (n>0){ 
             JOptionPane.showMessageDialog(null,"delete ok");

The update is similar. So, in this way, I use the doQuery2 method of Connectiondb.class only for the try-catch control of input, for example:

String art=txtformattedcf.getText().trim();
             String sqluss="select CF from Art where Art.CF='"+art+"'";
             ResultSet rsuss=Connectiondb.doQuery2(sqluss,true);
             if (rsuss.next()==false){
                 throw new SQLException();
             }

I use the second parameter when I have to put the data in jtable etc., but it's not important here.

Is there a better way to manage all of this, such as adding other methods in Connectiondb? I use these insert/delete in the GUIs many times and I think management is not efficient, maybe because I didn't create the classes for the date. For example, a class to take the information of a person (with get, set) of a company or what I need. I didn't do that because I treat the data directly to the database.

For example, is there a way to do this code

Connection conn=Connectiondb.loadDriver_doConnection_backConn();
     Statement sent=conn.createStatement();
     int n =sent.executeUpdate(sqlelimart);
     if (n>0){ 
         JOptionPane.showMessageDialog(null,"delete ok");

inside of a method Connectiondb where I give the SQL String (without then rewrite this code every time in each GUI)? I created the method Connectiondb.loadDriver_doConnection_backConn() because I always treat the data in the buttons GUI and not in the Connectiondb class. This is also the same for the insert and update, but now I can't find a right way.

For the preparedStatement method I'm writing this code:

static void SetString(PreparedStatement ps, Integer[] indexstring, JTextField[] textstring){
    for (int i=0;i<indexstring.length;i++)
        try {
            ps.setString(indexstring[i], textstring[i].getText().trim());
        } catch (SQLException e) {
            //msg with error
        }
}

static void SetInt(PreparedStatement ps, Integer[] indexint, JTextField[] textint){
    for (int i=0;i<indexint.length;i++)
        try {
            ps.setInt(indexint[i], Integer.parseInt(textint[i].getText().trim()));
        } catch (SQLException e) {
            //msg with error
        }
}

static void SetDouble(PreparedStatement ps, Integer[] indexdouble, JTextField[] textdouble){
    //etc
}

 static void SetDate(PreparedStatement ps, Integer[] indexdate, JTextField[] textdate){
     //etc
 }

There is a better way to do this? (In this way I have to write other 4 methods for the JFormattedTextField)

Answer 1

I guess I cover just a small part leaving some work for others

Sorry, but when I see this

public static void loadDriver_doConnection(){
    try {
        Class.forName(jdbc_driver);

        try { conn =  DriverManager.getConnection(url1,"root","pswww");
        }
        catch(SQLException es)
        { //es.printStackTrace();

        }

    }
    catch (ClassNotFoundException ex) { 
        //ex.printStackTrace();

    }
}

... so :(

• Can't you format it? Your IDE would do it for free, and so would I. Not because of the CR but you'll have to read it one day.
• //es.printStackTrace(); is this a comment? OK, I see, you could print the stack trace. And this is the very least thing you must when an exception occurs.
• The alternative is your program never complaining any problem and pretending to work. Then you can happily hunt down what happend.
• OK, as the DB is rather central, probably nothing at all would work and you could find it.

My rules for exception handling are long but simple:

• If you think you can handle it, think again.
• If you still think you can handle it, do it properly.
• Proper handling doesn't mean swallowing.
• Printing e.getMessage is nearly as bad.
• e.printStackTrace or logging is much better, but still mostly wrong.
• Ask yourself the question, what can your program when this failed.
• Can the user do anything?
• Can the user do retry? Does it make sense? How many times?
• Most exception should be simply declared as throws ... and you're done.
• If it's impossible, consider throw new MyException(e).

---

Is there a better way to manage all of this, such as adding other methods in Connectiondb?

Why would you do it (unless it's very short)?

---

ps.setString(1, txtformattedcf.getText().trim());

You're talking to the DB directly from the form... this works fine as long as the project stays small and you don't need to do anything with the values. The correct way is to create an object representing the Art. The GUI would update the object and the object would get stored.

This sounds like twice as much work (and sometimes it is), but usually, you do much more things than just this. And there are also tools allowing to map classes onto tables rather automatically saving you half of the work. The other part is tougher (at least I'm unaware of any helpful tool for this), but something can be done.

---

You could save yourself all the trimming by running once over the whole component tree. This would trim the strings directly in the form, which is different from what you're doing, but this doesn't matter.

---

I see I don't really know what you're asking... given that I see I wrote already too much. ;)

---

If I were to store data from a form directly to the DB (Heaven forbid!), then I'd go for reflection and eliminate all the code. This would make using systematic naming necessary, so I could rely on TextField named cf to be mapped to column named cf (or some more complicated convention) nearly always and solve the remaining cases via annotations.

Actually, I've just started writing something similar for filling my forms from objects and vice versa. The reflection is slow, but it should suffice even for the fastest typest in the world.

Answer 2

Apart from everything that's already been said:

• If you're already using prepared statements once, do it all the time.
• if (rsuss.next()==false){ is usually if (!rsuss.next())
• rs is a reused globals/field of your class? I would rather use local variables.
• And all the static looks like it should be unnecessary as well.

If you don't go the reflection route, consider extracting all duplicated code into functions/methods. I.e. for ps.setString(12, txtformattedtel.getText().trim()); create setTrimmedField(ps, 12, txtformattedtel);, or something a long those lines. Goes a long way to reduce repetition:

setTrimmedField(ps, index, GUI foo) {
  ps.setString(index, foo.getText().trim());
}

If all of those look the same, maybe setTrimmedFields(ps, 11, asArray(txtemail, ...)):

public void setTrimmedFields(ps, index, List<GUI> foos) {
  for (GUI bar in foos)
    setTrimmedField(ps, index, foos.get(index++));
}

You can remove a lot of static code with more dynamic variants.

Edit: Regarding the added code for the prepared statements:

From the documentation the SQLException only occurs in two situations, one being a programmer error (mismatched index for the query) and the other one if the statement was closed. Therefore I suggest not doing such a fine-grained exception handling, but rather wrapping the whole block for setting the prepared statement parameters in one catch block (with one error message instead of multiple ones).

Other than that I think using native arrays is not the nicest way. Thinking about it, what you could also do is using a Map with the index as the key and the widget as the value, then iterate over all tuples. That would make the interface for each of the methods a bit safer, since you basically removed one possibility for errors.