Combining a JFrame, AbstractTableModel, and SQL Query

Building on my previous questions. I've managed to make a GUI that queries a SQL database and displays the result in a table. Christ, it's a lot of work for such a simple activity.

The code below works. Does it adhere to MVC conventions? If I understand correctly, MainFrame and CustomTable are the "View" classes. They format and present data in the GUI. Student and its subclasses are the "Model" classes. They hold the data retrieved from the database. Misc is the "Controller" class. It queries the database, instantiates Model class objects to hold the data, then passes the Model data to the View classes. Do I have that right?

Apologies in advance for the null layout and bad error handling. They're two area I haven't studied sufficiently.

There are 4 classes I'm not posting. Student and its three subclasses Undergraduate, Graduate, and PartTime. Each class has properties that correspond to columns in a database table. And each instantiated object should correspond to a row in the table.

MainFrame class:

```
package kft1task4;
import javax.swing.*;
import java.awt.event.*;
import java.sql.SQLException;
import javax.swing.JScrollPane;

public class MainFrame extends JFrame implements ActionListener {
JPanel pane = new JPanel();
JButton butt = new JButton("Push Me To Update The Table");
CustomTable ct = new CustomTable();
JTable tab = new JTable(ct);

MainFrame() {
setVisible(true);
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
setBounds(1000,200,1500,1000);
pane.setLayout(null);
add(pane);
butt.setBounds(20,550,200,100);
butt.addActionListener(this);
pane.add(butt);

JScrollPane jsp = new JScrollPane(tab);
jsp.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
jsp.setBounds(20,50,1460,500);
pane.add(jsp);
}

@Override
public void actionPerformed(ActionEvent e) {
try{
ct.reQuery();
}
catch (ClassNotFoundException f){System.out.println("Excepti

-

@Override
public void actionPerformed(ActionEvent e) {
    try {
        ct.reQuery();

Action listeners run on the Event dispatching thread and they should be fast. Otherwise the GUI will be irresponsive. Worker Threads and SwingWorker

-

conn.close();
stmt.close();
rs.close();

I guess they should be closed in the reverse order:

rs.close();
stmt.close();
conn.close();

Anyway, they should be closed in a finally block or with try-with-resources. See Guideline 1-2: Release resources in all cases in Secure Coding Guidelines for the Java Programming Language

-
The Misc.query method creates a new connection on every call. It could cache the Connection objects for lower latency.

-
The if-elseif-elseif structure in the Misc.query method looks too complicated. Results of rs.get* calls which are used in every case could have local variables:

while (rs.next()) {
    String classname = rs.getString("class");
    String firstname = rs.getString("nameFirst");
    String lastname = rs.getString("nameLast");
    ...
}

Anyway, I'd go with JPA or Hibernate, they do this object-relational mapping quite well.

-

return students.get(row).getClass().toString().replace("class kft1task4.", "");

This could be

return students.get(row).getClass().getSimpleName();

You could declare a getName() method in the Student class and override it in subclassess appropriately.

-
Instead of printing the same Exception string I'd try to show a useful error message to the user and log the stacktrace of the exception. See: log4j vs. System.out.println - logger advantages? (I suggest you SLF4J with Logback.)

-
A great comment from @tb-'s answer:


Do not extend JPanel, instead have a private JPanel
attribute and deal with it (Encapsulation).
There is no need to give access to all
JPanel methods for code dealing with a UserPanel
instance. If you extend, you are forced to stay with this forever,
if you encapsulate, you can change whenever you want without
taking care of something outside the class.

(Substitute JPane with JFrame.)

-
Fields should be private. (Item 13 of Effective Java 2nd Edition: Minimize the accessibility of classes and members)

-
I would use longer variable and method names than pane, ct etc. Longer names would make the code more readable since readers don't have to decode the abbreviations every time when they write/maintain the code and don't have to guess which abbreviation the author uses.

reQuery() also could be refresh().

(Clean Code by Robert C. Martin, Avoid Mental Mapping, p25)

-

JButton butt = new JButton("Push Me To Update The Table");

This kind of behaviour usually referred as refresh, I'd use that as text.

-
ArrayList reference types should be simply List. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

ArrayList students = new ArrayList();

Another one:

public static ArrayList query() ...

-

CustomTable() {
    // reQuery();
}

Constructors should be at (almost) the beginning of the class, then the other methods. (According to the Code Conventions for the Java Programming Language, 3.1.3 Class and Interface Declarations.)

-
Commented out code is bad:

CustomTable() {
    // reQuery();
}

Why are those two lines of code commented? Are they important? Were they left as
reminders for some imminent change? Or are they just cruft that someone commented-out
years ago and has simply not bothered to clean up.

Source: Robert C. Martin: Clean Code, Chapter 4: Comments, Commented-Out Code:

-
I'd give a more descriptive name to Misc which says the purpose of the class. (StudentDatabase, for example.)

-

String cl = "";
ResultSet rs = stmt.executeQuery("Select * from students");

while (rs.next()) {
    cl = rs.getString("class");

About the cl variable: Try to minimize the scope of local variables. It's not necessary to declare them at the beginning of the method, declare them where they are first used. (Effective Java, Second Edition, Item 45: Minimize the scope of local variables)

-
Comments on the closing curly braces are unnecessary and disturbing. Modern IDEs could highlight blocks.

} // end query method

“// …” comments at end of code block after } - good or bad?

StackExchange Code Review Q#28256, answer score: 5