Error when Selecting Table row from JTable with AbstractTableModel

Question

I have an ArrayList of type modules, each module has an arraylist of Assignments. I have written the following TableModel but when the something is selected in the table I have a problem and it causes my ArrayLists to be out of bounds. Here is my table model:

public class AssignmentsTableModel extends AbstractTableModel {
private ArrayList<Module> modules;
private static final String[] COLUMN_NAMES = {"Module Identifier", "Module Name", "Assignment Title", "Author", "Date Set", "Date Due", "Weighting"};

private int moduleID;
private int assignmentID;
private Module module;
private int totalNumberAssignments;

public AssignmentsTableModel(ArrayList<Module> modules) {
    moduleID = 0;
    assignmentID = 0;
    this.modules = modules;
    module = modules.get(moduleID);
    totalNumberAssignments = getRowCount();
}

public AssignmentsTableModel(Module module) {
    modules = new ArrayList<Module>();
    modules.add(module);
}

@Override
public int getRowCount() {
    int rowCount = 0;
    for(Module mod : modules){
        rowCount += mod.getAssignments().size();
    }
    return rowCount;
}

@Override
public int getColumnCount() {
    return COLUMN_NAMES.length;  //To change body of implemented methods use File | Settings | File Templates.
}

@Override
public Object getValueAt(int row, int column) {
    if(column%getColumnCount() == 0 && getRowCount() <= totalNumberAssignments){
        if(isAtLastAssignment(row, moduleID)){
            assignmentID = 0;
            moduleID += 1;
        }
    }

    module = modules.get(moduleID);
    ArrayList<Assignment> assignments = module.getAssignments();

    Assignment assignment = assignments.get(assignmentID);

    switch (column){
        case 0: return module.getIdentifier();
        case 1: return module.getTitle();
        case 2: return assignment.getTitle();
        case 3: return assignment.getAuthor();
        case 4: return assignment.getSet();
        case 5: return assignment.getDue();
        case 6: return assignment.getWeighting();
        default: return null;
    }


}

public String getColumnName(int columnIndex) {
    return COLUMN_NAMES[columnIndex];
}

public boolean isAtLastAssignment(int row, int moduleID){
    boolean atLast = false;
    ArrayList<Assignment> assignments = modules.get(moduleID).getAssignments();
    if(moduleID == 0){
        if(row == assignments.size()){
            atLast = true;
        }
    } else {
        int assignmentsSize = assignments.size() + getCurrentRowNumber(moduleID);
        if(row == assignmentsSize){
            atLast = true;
        }
    }

    return atLast;
}

public int getCurrentRowNumber(int moduleID){
    int rowNumber = 0;

    for(int i = 0; i < moduleID; i++){
        rowNumber = modules.get(i).getAssignments().size()-1;
    }

    return rowNumber;
}
}

As you can see I store the moduleID as global variables which is fine when its first started, but once something is selected it uses the last values. What else can I do to stop this from happening?

Answer 1

The TableModel method getValueAt() is called by the table renderer each time it wants to get the value of the cell so that it can render it. It's not (directly or reliably) related to you clicking on the table row and having the table mark the row as selected.

In other words, you don't want to be mutating the state of your table model as a result of calling the getValueAt() method.

It would be easiest if your getValueAt() method use the row as the index into your array. However, it looks like you have a variable number of rows for each module. In that case, if you can't algorithmically generate the right index, you could "flatten" the data once in the constructor, and simplify the look-up code.

That is, your table model contains:

private List<RowData> rowData

and an inner class which contains the row data:

private static class RowData
{
  private Moddule;
  private Assignment assignment;

  ...
}

And in your constructor:

public AssignmentsTableModel(List<Module> modules)
{
  this.rowData = new ArrayList<Module>();

  for (Module module : modules)
  {
    for (Assignment assignment : module.getAssignments())
    {
      rowData.add(new RowData(module, assignment));
    }
  }
}

This simplifies your getValueAt() method:

@Override
public Object getValueAt(int row, int column) {
    final RowData row = this.rowData.get(row);

    switch (column){
        case 0: return row.getModule().getIdentifier();
        case 1: return row.getModule().getTitle();
        case 2: return row.getAssignment().getTitle();
        case 3: return row.getAssignment().Author();
        case 4: return row.getAssignment().getSet();
        case 5: return row.getAssignment().getDue();
        case 6: return row.getAssignment().getWeighting();
        default: return null;
    }
}