JComboBox as Jtable CellEditor with Overriden stopCellEditing modifies wrong table cell

Question

I have a custom JTable with a custom TableModel using a JComboBox as a cell editor. The ComboBox also has a custom ComboBoxModel The ComboBox model holds multiple fields that will be used to update the data behind the JTable and afterwards update a database.

The following is a simple example to show the problem I am encountering. Steps to reproduce:

Click on an cell
Select a value from the ComboBox drop-down list
Click on a different cell
Click back on the first selected cell

The second cell will get the value from the first one.

Why is this happening? Why does the ComboBox model change before stopCellEditing exists?

import javax.swing.DefaultCellEditor;
import javax.swing.DefaultComboBoxModel;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JScrollPane;
import javax.swing.JTabbedPane;
import javax.swing.JTable;
import javax.swing.table.DefaultTableModel;

public class TestComboCellEditor {

    public static void main(String[] args) {

        TestComboCellEditor test = new TestComboCellEditor();
        test.go();
    }

    public void go() {

        //create the frame
        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        // create and add a tabbed pane to the frame
        JTabbedPane tabbedPane = new JTabbedPane();
        frame.getContentPane().add(tabbedPane);
        //create a table and add it to a scroll pane in a new tab
        final JTable table = new JTable(new DefaultTableModel(new Object[]{"A", "B"}, 5));
        JScrollPane scrollPane = new JScrollPane(table);
        tabbedPane.addTab("test", scrollPane);

        // create a simple JComboBox and set is as table cell editor on column A
        Object[] comboElements = {"aaaaa1", "aaaaaa2", "b"};
        final JComboBox comboBox = new JComboBox(comboElements);
        comboBox.setEditable(true);
        table.getColumn("A").setCellEditor(new DefaultCellEditor(comboBox) {
            @Override
            public boolean stopCellEditing() {
                if (comboBox.isEditable()) {
                    DefaultComboBoxModel comboModel = (DefaultComboBoxModel) comboBox.getModel();
                    String selectedItem = (String) comboModel.getSelectedItem();
                    int selectedIndex = comboModel.getIndexOf(selectedItem);
                    if (!(selectedIndex == -1)) {
                        // the selected item exists as an Option inside the ComboBox
                        DefaultTableModel tableModel = (DefaultTableModel) table.getModel();
                        int selectedRow = table.getSelectedRow();
                        int selectedColumn = table.getSelectedColumn();
                        tableModel.setValueAt(selectedItem, selectedRow, selectedColumn);
                    } else if (selectedItem != null) {
                        // missing code - adding new info to a custom JComboBox model and to alter info inside a custom table model
                    }
                }
                return super.stopCellEditing();
            }
        });

        // pack and show frame
        frame.pack();
        frame.setVisible(true);

    }
}

the implementation is invalid: an editor **must not** change the view/model that's calling it - its sole responsibility is notifying when it's done with editing and keeping the edited value around (for its client to access) — kleopatra, Nov 14 '13 at 12:45
my first try was this: http://stackoverflow.com/questions/19938204/return-the-focus-to-jcombobox-inside-a-jtable-after-showoptiondialog/19938451#19938451 but encountered other issues so i got recommended to go this way. I been looking around for a best practice to implement what i want but couldn't find something to use. If you could point to a link with an example it would be great. — Alex Burdusel, Nov 14 '13 at 12:48

score 4 · Answer 1 · answered Nov 14 '13 at 17:29

4

Here is an approach that keeps all the code in the editor:

import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import javax.swing.table.*;

public class TestComboCellEditor {

    public static void main(String[] args) {

        TestComboCellEditor test = new TestComboCellEditor();
        test.go();
    }

    public void go() {

        //create the frame
        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        // create and add a tabbed pane to the frame
        JTabbedPane tabbedPane = new JTabbedPane();
        frame.getContentPane().add(tabbedPane);
        //create a table and add it to a scroll pane in a new tab
        final JTable table = new JTable(new DefaultTableModel(new Object[]{"A", "B"}, 5));
        JScrollPane scrollPane = new JScrollPane(table);
        tabbedPane.addTab("test", scrollPane);

        // create a simple JComboBox and set is as table cell editor on column A
        Object[] comboElements = {"aaaaa1", "aaaaaa2", "b"};
        final JComboBox comboBox = new JComboBox(comboElements);
        comboBox.setEditable(true);
        table.getColumn("A").setCellEditor(new DefaultCellEditor(comboBox)
        {
            private Object originalValue;

            @Override
            public Component getTableCellEditorComponent(JTable table, Object value, boolean isSelected, int row, int column)
            {
                originalValue = value;
                return super.getTableCellEditorComponent(table, value, isSelected, row, column);
            }

            @Override
            public boolean stopCellEditing()
            {
                JComboBox comboBox = (JComboBox)getComponent();
                DefaultComboBoxModel comboModel = (DefaultComboBoxModel) comboBox.getModel();
                Object editingValue = getCellEditorValue();

                //  Needed because your TableModel is empty

                if (editingValue == null)
                    return super.stopCellEditing();

                int selectedIndex = comboModel.getIndexOf(editingValue);

                //  Selecting item from model

                if (! (selectedIndex == -1))
                    return super.stopCellEditing();

                //  Confirm addition of new value

                int result = JOptionPane.showConfirmDialog(
                    comboBox.getParent(),
                    "Add (" + editingValue + ") to table?",
                    "Update Model",
                    JOptionPane.YES_NO_OPTION);

                if (result == JOptionPane.YES_OPTION)
                {
                    comboBox.addItem(editingValue);
                    return super.stopCellEditing();
                }
                else
                {
                    comboBox.removeItem(editingValue);
                     comboBox.setSelectedItem(originalValue);
                    return false;
                }
            }
        });

        // pack and show frame
        frame.pack();
        frame.setVisible(true);

    }
}

answered Nov 14 '13 at 17:29

camickr

321,443
19
166
288

1

that's near (and +1 so far!) - except a) (guessing a bit) resetting the original value when the requirement seems to be to go on with the edit b) handling complexer objects. Without the setSelectedItem(original) in the else block, the optionpane shows up twice: the culprit is the somewhat weird ui/combo behaviour which fires an artificial actionEvent with "comboBoxEdited" .. – kleopatra Nov 14 '13 at 17:38
1

I don't really want all the code in the editor. I just tried it like that because I couldn't find a better solution. What I am looking for is a good practice that will make the code easy to maintain. I am currently trying a solution with custom renderers to extract only the info I want to display from the custom model, with the dialog inside the cellEditor (as you suggested). This way when the ComboBox will be using the same data structure that the JTableModel is holding and will pass it directly when you select it. The JTable will be using similar renderer. Let me know what you think of this. – Alex Burdusel Nov 14 '13 at 20:24
I think I don't understand your requirement. I answered your original question. – camickr Nov 14 '13 at 21:05
You are right, your solution does answer my original question. :) While discussing with kleopatra on my answer below I presented a wider view of the implementation I am looking for (in the comments). Nevertheless, your solution is the answer for my initial question. – Alex Burdusel Nov 14 '13 at 21:16
kleopatra is right regarding the fact that the option pane shows up twice. Reproduce as follows: run the program; add a value that is not contained in the list at the first cell editing. Can't figure out why it's doing this. I added setSelectedItem(originalValue), but now it doesn't keep the cell editing, just loses focus. – Alex Burdusel Nov 17 '13 at 11:02
@Burfee, you are allowed to do a little debugging you know. The answer is simple, its just another condition on the if statement. We are not here to spoon feed you answers all the time. Only point you in the right direction, which I did. Since you no longer accept the answer, I'll let you do a little work on your own. – camickr Nov 17 '13 at 18:51
@camickr I've been doing debugging since you posted the solution, but couldn't figure out the bug. I just unchecked it because it's not working and considered it incomplete. Sorry if you found it offending. I am not requesting you to feed answers all the time. I am just putting feedback here in case someone will come with an answer sometime. Again, sorry for the inconvenience. – Alex Burdusel Nov 17 '13 at 19:09
@Burfee, `I've been doing debugging since you posted the solution, but couldn't figure out the bug.` what is displayed in the "()" when the option pane is displayed? – camickr Nov 17 '13 at 19:14
nothing, it keeps the old null value. For some odd reasons it seems to invoke in a loop the stopCellEditing and stops when it meets one of the if conditions that points to super.stopCellEditing. When it hits return false it does not return to cell editing but runs the stopCellEditing again. – Alex Burdusel Nov 17 '13 at 19:17
@Burfee, `nothing, it keeps the old null value.` - How do you know it is null? The word "null" is usually displayed when you print a null variable. Does the code not have a check for null which will prevent the option pane from displaying? – camickr Nov 17 '13 at 19:24
First the edditingValue is null, than, after it invokes super.stopCellEditing, it becomes an empty string. I tried adding the empty string as an or in the condition for null, but than I get other loops and the problem it's still not fixed. Just try to run the code, click the first cell, than click the second one without filling anything. – Alex Burdusel Nov 17 '13 at 19:33
`if (editingValue == null || editingValue.toString().length() == 0) ` - works for me using JDK7 on Windows 7. – camickr Nov 17 '13 at 19:41
if you click No on the OptionPane ConfirmDialog it does not return to cell editing. It's because it runs again stopCellEditing when you click no, and can't figure out why it's doing this, since it returned false the first time. JDK 7 on Ubuntu 12.04 x64. – Alex Burdusel Nov 17 '13 at 19:48
@Burfee Agreed, this approach isn't working. stopCellEditing() should only be called once with the value from the editor. You could probably reduce the complexity of the code by making sure that all cells are initially populated with a valid value (ie a value found in the combo box). But there is still the problem of keeping the cell in editing mode. You could probably invoke the editCellAt() method to place the cell back into editing mode but this seems like a bigger hack. – camickr Nov 17 '13 at 21:46

Alex Burdusel · Answer 2 · 2013-11-14T15:45:30.457

Ok, I've done some changes and I think I got something working. If it is not the best practice and you got a better implementation, please post an answer.

Edit: Do not follow the example below! Following kleopatra's comments it is a wrong implementation. I'm leaving it here so you know how not to do it.

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.DefaultCellEditor;
import javax.swing.DefaultComboBoxModel;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JScrollPane;
import javax.swing.JTabbedPane;
import javax.swing.JTable;
import javax.swing.table.DefaultTableModel;

public class TestComboCellEditor {

    public static void main(String[] args) {

        TestComboCellEditor test = new TestComboCellEditor();
        test.go();
    }

    public void go() {

        //create the frame
        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        // create and add a tabbed pane to the frame
        JTabbedPane tabbedPane = new JTabbedPane();
        frame.getContentPane().add(tabbedPane);
        //create a table and add it to a scroll pane in a new tab
        final JTable table = new JTable(new DefaultTableModel(new Object[]{"A", "B"}, 5));
        JScrollPane scrollPane = new JScrollPane(table);
        tabbedPane.addTab("test", scrollPane);

        // create a simple JComboBox and set is as table cell editor on column A
        Object[] comboElements = {"aaaaa1", "aaaaaa2", "b"};
        final JComboBox comboBox = new JComboBox(comboElements);
        comboBox.setEditable(true);
        table.getColumn("A").setCellEditor(new DefaultCellEditor(comboBox) {
            @Override
            public boolean stopCellEditing() {
                if (comboBox.isEditable()) {
                    DefaultComboBoxModel comboModel = (DefaultComboBoxModel) comboBox.getModel();
                    String selectedItem = (String) comboModel.getSelectedItem();
                    int selectedIndex = comboModel.getIndexOf(selectedItem);
                    if (!(selectedIndex == -1)) {
                        comboBox.actionPerformed(new ActionEvent(this, selectedIndex, "blabla"));
                    } else if (selectedItem != null) {
                        // missing code - adding new info to a custom JComboBox model and to alter info inside a custom table model
                    }
                }
                return super.stopCellEditing();
            }
        });

        comboBox.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                // the selected item exists as an Option inside the ComboBox
                if (e.getActionCommand().equals("blabla")) {
                    DefaultComboBoxModel comboModel = (DefaultComboBoxModel) comboBox.getModel();
                    String selectedItem = (String) comboModel.getSelectedItem();
                    DefaultTableModel tableModel = (DefaultTableModel) table.getModel();
                    int selectedRow = table.getSelectedRow();
                    int selectedColumn = table.getSelectedColumn();
                    tableModel.setValueAt(selectedItem, selectedRow, selectedColumn);
                }
            }
        });

        // pack and show frame
        frame.pack();
        frame.setVisible(true);

    }
}

no, that's wrong - **don't** interfere with JTable internal updates, neither in the editor nor in a listener. — kleopatra, Nov 14 '13 at 14:04
Thanks for the hints. From where should I do the updates when the editing is finished? — Alex Burdusel, Nov 14 '13 at 14:08
I still don't quite understand why this here would help your _real_ issue - you want to either silently add the new item (to the combo's model) plus use it as the table's model or do neither and let the user choose again, right? — kleopatra, Nov 14 '13 at 14:15
I want that when the user fills in the combobox a value that is not in the list, to pop a dialog box that asks if they want to add this value to the list (i removed the dialog box from this example because i just wanted to focus on models). If they don't i want to return to editing the combobox. Also, after a value is selected in the combobox, i want to update the tablemodel, but not just the cell value, since the combobox model is holding other info regarding that value (like id for the database, etc) and i want to pass it to the table model so i can use it afterwards to update the database. — Alex Burdusel, Nov 14 '13 at 14:19
regarding the _not just the cell value_ - that smells like a suboptimal data model: let the cell contain the complete object (displayvalue, plus other info) and use custom renderers/editors. — kleopatra, Nov 14 '13 at 14:23
you hit a patch of dirt with your requirement .. the editor implementations (DefaultCellEditor and even worse the comboCellEditor in the comboBox) are very sub-optimal, so it's hard (read: I couldn't do it on-the-fly) to achieve. If you are pressed, going with whatever dirty hack you come up might indeed be the best option for now - doing it right later. — kleopatra, Nov 14 '13 at 15:32
Thanks for trying. I got some great hints from your comments anyway and will try a new implementation. Post your response whenever you got time for it, I am not pressed. — Alex Burdusel, Nov 14 '13 at 15:44

JComboBox as Jtable CellEditor with Overriden stopCellEditing modifies wrong table cell

2 Answers2

Linked