12

I have a JTable populated with a custom DataModel (pasted below) and when I call the populate() method, it appears to populate the table with duplicate data - each row is filled with the same value over and over again. However, on closer inspection (by simply println()ing the 'data' field), the data model isn't at fault - it holds correct data, in the format I expect. What gives?

import java.util.ArrayList;    
import javax.swing.table.AbstractTableModel;

@SuppressWarnings("serial") // we don't expect this app to ever use serialized classes.  EVER.
public class CollectionDataModel extends AbstractTableModel {
    private ArrayList<ArrayList<String>> data;

    public CollectionDataModel() {
        data = new ArrayList<ArrayList<String>>();
    }

    @Override
    public int getColumnCount() {
        if(data.isEmpty()) return 0;
        return data.get(0).size();
    }

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

    @Override
    public Object getValueAt(int rowIndex, int columnIndex) {
        if(rowIndex > getRowCount()) return null;
        if(columnIndex > getColumnCount()) return null;
        return data.get(rowIndex).get(columnIndex);
    }

    public void populate(Collection c) {
        data.clear();
        for(Item i : c.getItems()) {
            ArrayList<String> row = new ArrayList<String>();
            for(Property p : i.getProperties().values()) {
                row.add(p.toString());
            }
            data.add(row);
        }
        fireTableDataChanged();
    }

}
Adrian
  • 5,603
  • 8
  • 53
  • 85
Chris Browne
  • 1,582
  • 3
  • 15
  • 33

3 Answers3

10

Here's a complete example that may prove helpful. As the sample Map is unmodifiable, I refer you to @mKorbel's example on how to override isCellEditable() and setValueAt().

import java.awt.EventQueue;
import java.awt.GridLayout;
import java.util.Map;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.JTable;
import javax.swing.table.AbstractTableModel;

/** @see https://stackoverflow.com/questions/9132987 */
public class EnvTableTest extends JPanel {

    public EnvTableTest() {
        this.setLayout(new GridLayout());
        this.add(new JScrollPane(new JTable(new EnvDataModel())));
    }

    private static class EnvDataModel extends AbstractTableModel {

        private Map<String, String> data = System.getenv();
        private String[] keys;

        public EnvDataModel() {
            keys = data.keySet().toArray(new String[data.size()]);
        }

        @Override
        public String getColumnName(int col) {
            if (col == 0) {
                return "Key";
            } else {
                return "Value";
            }
        }

        @Override
        public int getColumnCount() {
            return 2;
        }

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

        @Override
        public Object getValueAt(int row, int col) {
            if (col == 0) {
                return keys[row];
            } else {
                return data.get(keys[row]);
            }
        }
    }

    private void display() {
        JFrame f = new JFrame("EnvTableTest");
        f.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        f.add(this);
        f.pack();
        f.setLocationRelativeTo(null);
        f.setVisible(true);
    }

    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {

            @Override
            public void run() {
                new EnvTableTest().display();
            }
        });
    }
}
Community
  • 1
  • 1
trashgod
  • 203,806
  • 29
  • 246
  • 1,045
  • This example assumes I know (at least) the number of columns upfront. My code does not require foreknowledge of column count (for a reason). Thank you for your help nonetheless. I will implement the missing methods and rearrange my datatypes thanks to this answer. Accepting in lieu of anything better, going to use the advice given here to build my own solution. Thanks again. – Chris Browne Feb 03 '12 at 20:07
  • Excellent. Indeed, my example was contrived to be minimal, but it may serve as an [sscce](http://sscce.org/) foundation for any future questions that may arise. Also consider `List> data = new ArrayList>()`. Looking closer, I see your column count is `data.get(0).size()`. Does it matter if the inner lists have differing lengths? – trashgod Feb 03 '12 at 20:27
  • Good point, the inner lists shouldn't have different lengths but may do (in the case of corrupt data being loaded, I do have safeguards against that but it's better to be safe than sorry I suppose) - I shall fix that. Also, thank you for your tips on List vs ArrayList, I think it's obvious I'm new to OOP! Thanks again for your help! :) – Chris Browne Feb 04 '12 at 18:40
3

You could try to make the changes of populate more atomic.

public void populate(Collection c) {
    ArrayList<ArrayList<String>> data2 = new  ArrayList<ArrayList<String>>();
    for(Item i : c.getItems()) {
        ArrayList<String> row = new ArrayList<String>();
        for(Property p : i.getProperties().values()) {
            row.add(p.toString());
        }
        data2.add(row);
    }
    data = data2;
    fireTableDataChanged();
}

I am guessing that populate is called again before a prior populate call finished. And probably c is changed during its iteration.

Hovercraft Full Of Eels
  • 283,665
  • 25
  • 256
  • 373
Joop Eggen
  • 107,315
  • 7
  • 83
  • 138
  • 1
    I'm sorry, but how would this help? – Hovercraft Full Of Eels Feb 03 '12 at 17:56
  • 2
    I don't see what it would change. Everything in Swing is done in the event dispatch thread, so the table has no way to see any intermediate state. – JB Nizet Feb 03 '12 at 17:58
  • `populate` does not relate to the event thread. It probably is called whenever a cell is changed/added. That leaves the model in disarray, showing only a part. But more seriously, when the collection `c` is traversed when a change to the collection occurs, one may get things like duplicate values. – Joop Eggen Feb 03 '12 at 18:16
  • but I don't get duplicate values in data, I said that in my question - I only get duplicate values when the JTable is displayed, the actual data variable, when printed, is intact. It's the difference between the 'data' variable and what the JTable shows that I'm trying to debug. – Chris Browne Feb 03 '12 at 18:19
  • @HovercraftFullOfEels you are right, the code solves an _other_ concurrency fault; but the text mentions the fallacy of naively using `c` under concurrency. – Joop Eggen Feb 03 '12 at 18:23
  • @ChrisBrowne sorry, -1 deserved. Could you give my code a try? Maybe even do `public synchronized void populate`? I still think that repeatedly doing data.clear and refilling data has something to do with it. Simply logging would help to debug. – Joop Eggen Feb 03 '12 at 18:33
  • Any suggestions on what to log, other than 'data'? I did give your code a try, it didn't improve the situation, sorry. – Chris Browne Feb 03 '12 at 18:39
2

1) your TableModel is un_completed, I miss there lots or required methods for JTable's life_cycle, starting with TableHeader etc.

2) since there are lots of AbstactTableModels based on HashMap, I'd suggest to return arrays type implemented in API directly

Vector<Vector<Object or String>> data; 

String[][] or Object[][] 

instead of

ArrayList<ArrayList<String>> data;

simple explanations is that XxxList returs column and Vector or String[] returns Row

3) I'd suggest to use DefaultTableModel directly then you'll never need to solve duplicates or missed column/row

mKorbel
  • 109,525
  • 20
  • 134
  • 319
  • I'm terribly sorry, but I don't really understand your answer. Are you saying I'm missing methods from my TableModel? If so, which methods, specifically? I'm not getting any warnings or errors about unimplemented methods... – Chris Browne Feb 03 '12 at 18:13
  • hmmm wait minute I have to look for AbstractTableModel based on Vector on this forum, wrote AbstractTableModel (override methods from DefaultTableModel) not easy job and required basic knowledge about JTable and DefaultTableModel – mKorbel Feb 03 '12 at 18:17
  • OK. I'm not really following you, but it sounds like you're going to try and update your answer with more information so I'll check back in a few minutes. Thanks for trying to help me :) – Chris Browne Feb 03 '12 at 18:20
  • are you suggesting I create a custom renderer for the AbstractTableModel I created? Is this really necessary? – Chris Browne Feb 03 '12 at 18:25
  • Renderer has nothing to do with Model on the bottom is AbstractTableModel, with minimum methods and events fired – mKorbel Feb 03 '12 at 18:28
  • I'm really sorry, I'm finding it very difficult to understand you. I don't get any warnings for the AbstractTableModel I created, are you saying the extra methods in the one you linked to are necessary but not in the interface? I find that a little difficult to believe, which is why I'm inclined to believe I'm simply misunderstanding you. – Chris Browne Feb 03 '12 at 18:31
  • again before thinking about override methods from DefaultTableModel into own AbstractTableModel you have to know more than something about JTable / how TableModel works – mKorbel Feb 03 '12 at 18:41
  • That last comment was unhelpful and patronising. Please keep your comments on-topic and constructive in future. – Chris Browne Feb 03 '12 at 18:44
  • simple I tried to be on-topic, but you're mixing methods, too hard starting from the end – mKorbel Feb 03 '12 at 19:15
  • @mKorbel: +1 for `DefaultTableModel` as an example of implementing `AbstractTableModel` correctly. I took the liberty of citing your [example](http://stackoverflow.com/a/6656279/714968) in my [answer](http://stackoverflow.com/a/9134371/230513). – trashgod Feb 03 '12 at 19:32
  • @ChrisBrowne: I'm certain that mKorbel's comment was meant to be helpful. Native language may separate us, but we can share a common _lingua Java_. – trashgod Feb 03 '12 at 19:41