Combining a JFrame, AbstractTableModel, and SQL Query

Question

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("Exception");}
 catch (InstantiationException f){System.out.println("Exception");}
 catch (IllegalAccessException f){System.out.println("Exception");}
 catch (SQLException f){System.out.println("Exception");} 
 } 
}

CustomTable class:

package kft1task4;
import javax.swing.table.AbstractTableModel;
import java.util.ArrayList;
import java.sql.SQLException;
public class CustomTable extends AbstractTableModel { 
String[] colName = {"Classification", "Student ID", "First Name", "Last Name", "Status", "Mentor", "GPA", "Credit Hours", "Level", "Thesis Titile", "Thesis Advisor", "Company"};
ArrayList<Student> students = new ArrayList<Student>();
public String getColumnName(int col){
 return colName[col];
}
public int getColumnCount(){
 return colName.length;
}
public int getRowCount(){
 return students.size();
}
@Override
public String getValueAt(int row, int col){
 int j = 10;
 switch (col){
 case 0: return students.get(row).getClass().toString().replace("class kft1task4.", "");
 case 1: return Integer.toString(students.get(row).studentID);
 case 2: return students.get(row).nameFirst;
 case 3: return students.get(row).nameLast;
 case 4: return students.get(row).status;
 case 5: return students.get(row).mentor;
 case 6: return Double.toString(students.get(row).gpa);
 case 7: return Integer.toString(students.get(row).creditHours);
 case 8: if( students.get(row) instanceof Undergraduate){Undergraduate st = (Undergraduate)students.get(row); return st.level;} else {return null;}
 case 9: if( students.get(row) instanceof Graduate){Graduate st = (Graduate)students.get(row); return st.thesisTitle;} else {return null;}
 case 10: if( students.get(row) instanceof Graduate){Graduate st = (Graduate)students.get(row); return st.thesisAdvisor;} else {return null;}
 case 11: if( students.get(row) instanceof PartTime){PartTime st = (PartTime)students.get(row); return st.company;} else {return null;}
 default: return "Default"; 
 }
}
public boolean isCellEditable(int col, int row){
 return true;
}
@Override 
public void setValueAt(Object s, int row, int col){
 fireTableDataChanged();
}
public void reQuery() throws ClassNotFoundException, InstantiationException, IllegalAccessException, SQLException{
 try{
 students.clear();
 students = Misc.query();
 }
 catch (ClassNotFoundException f){System.out.println("Exception");}
 catch (InstantiationException f){System.out.println("Exception");}
 catch (IllegalAccessException f){System.out.println("Exception");}
 catch (SQLException f){System.out.println("Exception");}
 fireTableDataChanged();
 } 
 CustomTable() {
 //reQuery();
 }
 }

Misc class:

package kft1task4;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
public class Misc {
Misc(){
}
 public static ArrayList<Student> query() throws ClassNotFoundException, InstantiationException, IllegalAccessException, SQLException{ 
 ArrayList<Student> studs = new ArrayList<Student>();
 Connection conn = DriverManager.getConnection("jdbc:mysql://localhost/kft1task4?" + "user=root&password=");
 Statement stmt = conn.createStatement();
 String cl = "";
 ResultSet rs = stmt.executeQuery("Select * from students");
 while(rs.next()){
 cl = rs.getString("class"); 
 if(cl.equals("Undergraduate")){
 studs.add(new Undergraduate(rs.getString("nameFirst"),rs.getString("nameLast"),rs.getInt("student_id"),rs.getDouble("gpa"),rs.getString("status"),rs.getString("mentor"),rs.getInt("creditHours"),rs.getString("level")));
 }
 else if(cl.equals("Graduate")){
 studs.add(new Graduate(rs.getString("nameFirst"),rs.getString("nameLast"),rs.getInt("student_id"),rs.getDouble("gpa"),rs.getString("status"),rs.getString("mentor"),rs.getInt("creditHours"), rs.getString("thesisTitle"),rs.getString("thesisAdvisor")));
 }
 else if(cl.equals("PartTime")){
 studs.add(new PartTime(rs.getString("nameFirst"),rs.getString("nameLast"),rs.getInt("student_id"),rs.getDouble("gpa"),rs.getString("status"),rs.getString("mentor"),rs.getInt("creditHours"),rs.getString("Company")));
 }
 }
 conn.close();
 stmt.close();
 rs.close();
 return studs;
 } //end query method 
}

Answer 1

1. @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

2. 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

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

4. 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.

5. 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.

6. 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.)

7. 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.)

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

9. 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)

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

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

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

    ArrayList<Student> students = new ArrayList<Student>();
    

    Another one:

    public static ArrayList<Student> query() ...
    

12. 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.)

13. 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:

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

15. 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)

16. 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?