MySQL Java in JTable

This is an assignment posted here. (Part-B)

The aim here is to create a GUI in which the user can search for and insert data into a particular table of a MySQL database.

Is this an appropriate design or is it little overkill to make so many classes. The purpose of the DataClass is to make it possible to later change the back end database without making changes to the other classes.
Should there be two sub-classes of Entry, QueryEntry and InsertEntry that have their different roles? Or would that be too much?

Please, any feedback would be really helpful.

Entry Class:

```
package assign3;
/**
* Represents the abstraction for entries that are to be
* added to the database or searched/queried from the database.
*
* Provides methods for generating SQL statements based on the state
* of this Entry object.
*/
public class Entry {

/**
* Represents the JCombobox data which can be larger than, smaller than
* or equal to the population given in the field.
*/
public enum Population {
LARGER,
SMALLER,
EQUAL;
}

/**
* Represents the JCombobox data from the GUI, which can be exact match
* or partial match.
*/
public enum Match {
EXACT,
PARTIAL;
}

private String metropolis;
private String continent;
private String population;
private Population populationCriteria;
private Match matchCriteria;
// private static final String TABLE_NAME = "metropolises";

/**
* Construct an Entry/criteria to be searched/queried from
* the database. Any or all of the String parameters can be empty or
* null if they are not to be included in the query condition.
* @param metropolis name of the metropolis to be included in the query,
* can be left empty
* @param continent name of the continent to be included in the query,
* can be left empty
* @param population population to be included in the query, can be left empty
*

Connection management, and DataClass in general

DataClass is a poor name. What is the main purpose of this class? Find a better name, accordingly. In general, any name with "Class" in it is probably not a very good name for a class.

These members are immediately very suspicious:

private Connection connection;
private Statement stmt;
private ResultSet rs;
private ResultSetMetaData metadata;
private List columnNames;

That is:

• connection:

• Probably the class should only ever have one Connection. So it should be final. Of course, simply adding the final modifier will break the class as the current constructor is written. You'll need to refactor to make it compile.

• If you cannot obtain a connection in the constructor, the class will be useless anyway, so it should throw an exception. The bottom line is: you should be able to make this member final.

• stmt:

• First of all, consider using a PreparedStatement instead. That should speed up repeated execution of the same query string.

• I'm wondering if it makes sense to cache a prepared statement object at all. Ideally, the JDBC implementation should take care of the prepared statement pooling.

• In short, probably this should be a local variable instead, not a member

• rs should be local variable, not a member

• metadata and columnNames should be final, just like connection

Note that printing messages and stack trace to standard output is a bad practice.
You should log instead. I'm talking about this kind of thing:

} catch (SQLException se) {
    System.out.println("Insert Error");
    se.getStackTrace();
}

Make things final, when possible

Make everything final that you can, for example all these can be final:

private static String account = MyDBInfo.MYSQL_USERNAME;
private static String password = MyDBInfo.MYSQL_PASSWORD;
private static String server = MyDBInfo.MYSQL_DATABASE_SERVER;
private static String database = MyDBInfo.MYSQL_DATABASE_NAME;
private static String table = "metropolises";

There are many many more members in all the classes that you posted that could be final. Review the entire code and try to make all members final if possible. At the minimum, convert everything final where the build doesn't break. In cases when the build would break, consider if it makes sense to reassign the member:

• If yes, consider the possibility of making it a local variable (like Statement, above)

• If no, consider refactoring to make it final (like Connection, above)

Minor things

The semicolon is unnecessary in simple enums like this one:

public enum Population {
    LARGER,
    SMALLER,
    EQUAL;  // <- no need for this ";"
}

What's up with the pointless parentheses here? Just remove them:

public int getRowCount() {
    return(data.size());
}

This appears in many other places too. Review the entire code, remove pointless parentheses, they are just noise in the code.

This should be improved:

public static void main(String[] args) {
    try {
        UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
    } catch (Exception ignored) { }
    TableFrame tf = new TableFrame();
}

like this:

public static void main(String[] args) throws ClassNotFoundException, UnsupportedLookAndFeelException, InstantiationException, IllegalAccessException {
    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
    new TableFrame();
}

Because, why ignore the exception? Will the application actually function if an exception is thrown there? I doubt it. And if an exception is thrown but the code ignores it, how will you debug it?

Secondly, if you're not doing anything with the created TableFrame object,
then there's no point storing it in a variable.

Conclusion

I haven't reviewed your entire code. There might be a lot more that needs to be fixed. I suggest to apply these suggestions, and post the improved code in another question. Or you could also wait for more answers.

StackExchange Code Review Q#67215, answer score: 3