JTable duplicate values in row

Question

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();
    }

}

Hatfield

I don't see an obvious error in the code you've posted. If you don't get a decent answer soon, you may need to do a bit more debugging to try to isolate exactly where the error is occurring. Also, are you using a custom renderer? Could it be displaying the wrong data? Luck! – Riverside

nope, default renderer, I'll keep looking at it, thanks. At least I know I'm not going crazy! – Hatfield

Are you using cell render for the table? – Marchant

@ChrisBrowne: I make no claims as to extent or stability of your sanity. – Riverside

@HovercraftFullOfEels a sensible stance to take, it has a tendency to leak while I'm programming. – Hatfield

Consider creating and posting an sscce if no answer solves your problem. It will take a bit of work to create this, but it is often well worth it. – Riverside

Answer 1

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();
            }
        });
    }
}

Marola

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. – Hatfield

Excellent. Indeed, my example was contrived to be minimal, but it may serve as an sscce foundation for any future questions that may arise. Also consider List<List<String>> data = new ArrayList<List<String>>(). Looking closer, I see your column count is data.get(0).size(). Does it matter if the inner lists have differing lengths? – Marola

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! :) – Hatfield

Answer 2

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.

Ame

I'm sorry, but how would this help? – Riverside

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. – Metronome

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. – Ame

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. – Hatfield

@HovercraftFullOfEels you are right, the code solves an other concurrency fault; but the text mentions the fallacy of naively using c under concurrency. – Ame

@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. – Ame

Any suggestions on what to log, other than 'data'? I did give your code a try, it didn't improve the situation, sorry. – Hatfield

Answer 3

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

Chromatograph

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... – Hatfield

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 – Chromatograph

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 :) – Hatfield

are you suggesting I create a custom renderer for the AbstractTableModel I created? Is this really necessary? – Hatfield

Renderer has nothing to do with Model on the bottom is AbstractTableModel, with minimum methods and events fired – Chromatograph

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. – Hatfield

again before thinking about override methods from DefaultTableModel into own AbstractTableModel you have to know more than something about JTable / how TableModel works – Chromatograph

That last comment was unhelpful and patronising. Please keep your comments on-topic and constructive in future. – Hatfield

simple I tried to be on-topic, but you're mixing methods, too hard starting from the end – Chromatograph

@mKorbel: +1 for DefaultTableModel as an example of implementing AbstractTableModel correctly. I took the liberty of citing your example in my answer. – Marola

@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. – Marola