problems with editable JList

B

blmblm

Maybe one of the experts here can help .... Apologies in advance for
the length of this post; I've tried to make it as short as I could,
but oh well.

I'm trying to work up a prototype for a JList that supports some simple
editing operations: add a new element, replace an existing element,
remove a selected element, and do some simple reordering. My code
is working pretty well, *except* that when I try to set the selection
after an "add" or "remove", it doesn't quite work the way I want.

What I want:

After adding a new element, the new element is selected.

After removing an element, the preceding element is selected,
unless the removed element was the first one, in which case I want
the first one to be selected, or the only one, in which case I want
nothing selected.

How I'm trying to accomplish this:

In my extension of JList, I attach a ListDataListener to the list
data model, and in the listener's methods that deal with addition and
removal events I use setSelectedIndex() or clearSelection() to change
the selection to what I want. And it seems to work, based on what's
reported by debug prints -- *EXCEPT* that something [*] subsequently
changes the selection in a way that I don't want:

After an "add", I end up with a selected index of getModel().getSize(),
which of course is not valid.

After a "remove" of the first element, I end up with nothing selected,
even if there are elements remaining.

[*] Based on attempts to find out what's going on by adding a
lot of debug-print code and poring over source code for various
javax.swing classes, I *think* something in the ListDataListener in
plaf.basic.BasicListUI is responsible.

I've stripped down the prototype to what I think is just about the
smallest program that demonstrates the problem (well, okay, plus some
debug-print code), which appears below. Other than not being very short,
it's an SSCCE.

Am I going about this wrong? Have I tripped over a bug? Initial
development was done with Java 1.6.something, but the code (mis)behaves
the same way with 1.7.something.

Help appreciated!! the problem's not a show-stopper for my use case,
but it *is* annoying, and puzzling ....



/*
Attempt at an editable JList, supporting allow adding, removing, and
changing elements.

Works as desired except for (some) attempts to modify programmatically
what element is selected:

"add" should result in new item being selected (but does not).

"remove" should result in preceding element being selected if there is
one (and that works), or element 0 being selected if the removed element
was the one at index 0 (and that does not work).
*/

import java.awt.*;
import java.awt.event.*;

import javax.swing.*;
import javax.swing.event.*;


public class TryListMain extends JFrame {

// ---- variables ----

private static boolean DEBUG = true;

private DefaultListModel model;
private TryList list;

private static int counter = 0;

// ---- constructor ----

public TryListMain() {

super();

model = new DefaultListModel();

list = new TryList(model);

setLayout(new BorderLayout());

setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

add(new ButtonPanel(), BorderLayout.NORTH);
add(list, BorderLayout.CENTER);
}

// ---- methods ----

public TryList getList() {
return list;
}
public DefaultListModel getModel() {
return model;
}

public static void debugPrint(String msg) {
if (DEBUG) {
if (msg.length() > 0) {
System.out.println("DBG "+msg);
} else {
System.out.println();
}
}
}
public static void debugPrint() {
debugPrint("");
}

private String generateItem() {
++counter;
return "Element "+counter;
}

// ---- main ----

public static void main(String[] args) {

SwingUtilities.invokeLater(new Runnable() {
@Override
public void run() {

UIManager.put("Button.defaultButtonFollowsFocus", true);
TryListMain theFrame = new TryListMain();

System.out.println("data listeners:");
for (ListDataListener ll :
theFrame.getModel().getListDataListeners())
{
System.out.println(ll.getClass().getName());
}
System.out.println();

System.out.println("list selection listeners:");
for (ListSelectionListener ll :
theFrame.getList().getListSelectionListeners())
{
System.out.println(ll.getClass().getName());
}
System.out.println();

theFrame.setSize(new Dimension(400,200));
theFrame.setVisible(true);
}
});
}

// ---- classes ----

private class ButtonPanel extends JPanel {

private Action removeAction;
private Action replaceAction;
private Action newAction;
private Action showAction;

private void setActionsEnabled() {
int si = list.selectedIndex();
removeAction.setEnabled(si >= 0);
replaceAction.setEnabled(si >= 0);
}

public ButtonPanel() {

super(new FlowLayout());

removeAction = new AbstractAction("Remove") {
@Override
public void actionPerformed(ActionEvent e) {
int si = list.selectedIndex();
TryListMain.debugPrint();
TryListMain.debugPrint("== remove item at "+si);
if (si >= 0) {
model.remove(si);
} else {
// FIXME should never happen
System.err.println("no selected item to remove");
}
}
};

replaceAction = new AbstractAction("Replace") {
@Override
public void actionPerformed(ActionEvent e) {
int si = list.selectedIndex();
if (si >= 0) {
String item = generateItem();
TryListMain.debugPrint();
TryListMain.debugPrint("== replace item at "+si);
model.set(si, item);
} else {
// FIXME should never happen
System.err.println("no selected item to replace");
}
}
};

newAction = new AbstractAction("New") {
@Override
public void actionPerformed(ActionEvent e) {
String item = generateItem();
TryListMain.debugPrint();
TryListMain.debugPrint("== add new item");
model.addElement(item);
}
};

showAction = new AbstractAction("Show selected") {
@Override
public void actionPerformed(ActionEvent e) {
int si = list.selectedIndex();
if (si < 0) {
System.out.println("No selected value");
} else {
System.out.println(
String.format("Selected value (%d of %d) '%s'",
si+1, model.getSize(), model.get(si)));
}
}
};

list.addListSelectionListener(new ListSelectionListener() {
@Override
public void valueChanged(ListSelectionEvent e) {
setActionsEnabled();
}
});

setActionsEnabled();

add(new JButton(removeAction));
add(new JButton(replaceAction));
add(new JButton(newAction));
add(new JButton(showAction));
}
}
}

class TryList extends JList {

// ---- constructor ----

public TryList(DefaultListModel lModel) {
super(lModel);

getSelectionModel().setSelectionMode(
ListSelectionModel.SINGLE_SELECTION);

getModel().addListDataListener(new ListDataListener() {

@Override
public void intervalAdded(ListDataEvent e) {
printEvent("add", e);
setSelectedIndex(e.getIndex1());
ensureIndexIsVisible(e.getIndex1());
printNewSelection("add");
/*
HERE things seem okay -- desired element selected --
but then something changes the selection to
getModel().getSize(), which of course(?) is not valid
*/
TryListMain.debugPrint();
}

@Override
public void intervalRemoved(ListDataEvent e) {
printEvent("remove", e);
if (getModel().getSize() == 0) {
clearSelection();
} else {
int newSelect = Math.max(0, e.getIndex1()-1);
setSelectedIndex(newSelect);
ensureIndexIsVisible(newSelect);
}
printNewSelection("remove");
/*
HERE things seem okay -- desired element selected --
but then if the removed element was the first one
something changes the selection from (0) to ()
*/
TryListMain.debugPrint();
}

@Override
public void contentsChanged(ListDataEvent e) {
printEvent("change", e);
setSelectedIndex(e.getIndex1());
ensureIndexIsVisible(e.getIndex1());
printNewSelection("change");
TryListMain.debugPrint();
}
});

addListSelectionListener(new ListSelectionListener() {
@Override
public void valueChanged(ListSelectionEvent e) {
TryListMain.debugPrint(String.format(
"selection changed to %s, source %s",
selectedIndicesToString(),
e.getSource().getClass().getName()));
}
});
}

// ---- methods ----

// return -1 if nothing selected, or out of range
public int selectedIndex() {
int[] si = getSelectedIndices();
if ((si.length == 1) && (si[0] < getModel().getSize())) {
return si[0];
} else {
return -1;
}
}

private void printEvent(String name, ListDataEvent e) {
TryListMain.debugPrint();
TryListMain.debugPrint(
String.format("%s (%s, %d, %d), %d items (selected %s)",
name,
e.getSource().getClass().getName(),
e.getIndex0(), e.getIndex1(),
getModel().getSize(),
selectedIndicesToString()));
}

private void printNewSelection(String name) {
TryListMain.debugPrint(
String.format("after %s, selection %s",
name,
selectedIndicesToString()));
TryListMain.debugPrint();
}

private String selectedIndicesToString() {
StringBuffer sb = new StringBuffer();
for (int i : getSelectedIndices()) {
if (sb.length() > 0) sb.append(' ');
sb.append(i);
}
return "("+sb.toString()+")";
}
}
 
M

markspace

What I want:

After adding a new element, the new element is selected.
...
How I'm trying to accomplish this:

In my extension of JList, I attach a ListDataListener to the list
data model, and in the listener's methods that deal with addition and
removal events I use setSelectedIndex() or clearSelection() to change


I think your event listeners are tripping you up. They're probably
firing twice, or continuously until an error happens, after each time
you try to modify the list including modifications by the event listener
itself.

I made a simple program to modify a list, and it works fine. I didn't
use any event listeners. Try starting with this, see if you can use the
same template to get "Remove" working. "Add" seems to work fine.


/*
* To change this license header, choose License Headers in Project
Properties.
* To change this template file, choose Tools | Templates
* and open the template in the editor.
*/
package quicktest;

import java.awt.BorderLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.Box;
import javax.swing.DefaultListModel;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JList;
import javax.swing.SwingUtilities;

/**
*
* @author Brenden Towey
*/
public class JListTest
{

public static void main( String[] args )
{
SwingUtilities.invokeLater( new Runnable()
{
public void run()
{
JFrame frame = new JFrame();

final DefaultListModel listModel = new DefaultListModel();
Object[] test = { "One", "Two", "Three" };
for( Object li : test )
listModel.addElement( li );

final JList list = new JList( listModel );
frame.add( list );

Box vbox = Box.createVerticalBox();
JButton addButton = new JButton( "Add" );
vbox.add( addButton );
JButton removeButton = new JButton( "Remove" );
vbox.add( removeButton );
frame.add( vbox, BorderLayout.EAST );

addButton.addActionListener( new ActionListener()
{
int count = 0;
@Override
public void actionPerformed( ActionEvent e )
{
listModel.addElement( "Add "+ ++count );
list.setSelectedIndex( listModel.getSize()-1 );
}
} );

frame.pack();
frame.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE );
frame.setSize( 350, 350 );
frame.setLocationRelativeTo( null );
frame.setVisible( true );
}
} );
}
}
 
B

blmblm

I think your event listeners are tripping you up. They're probably
firing twice, or continuously until an error happens, after each time
you try to modify the list including modifications by the event listener
itself.


I'd have said not -- based on output of the even-more-debug-prints
version I'm reasonably confident that I'm not getting that kind of
recursion/loop, and anyway I only modify the selection in code to
process add/remove events, rather than in ....

But then again, both kinds of events (add/remove and selection changes)
are handled by code in plaf.basic.BasicListUI, and it does something
that, judging by comments in its code, is meant to avoid what you're
describing.

Be that as it may, though:

The big difference between your code and mine is that you moved the
code to set the selection into the button event listeners. I don't
know why I didn't think of that, but when I take that approach ....

Success!!

I'll post updated code later .... Real life is interfering right now!

Thanks!!!!
I made a simple program to modify a list, and it works fine. I didn't
use any event listeners. Try starting with this, see if you can use the
same template to get "Remove" working. "Add" seems to work fine.


/*
* To change this license header, choose License Headers in Project
Properties.
* To change this template file, choose Tools | Templates
* and open the template in the editor.
*/
package quicktest;

import java.awt.BorderLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.Box;
import javax.swing.DefaultListModel;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JList;
import javax.swing.SwingUtilities;

/**
*
* @author Brenden Towey
*/
public class JListTest
{

public static void main( String[] args )
{
SwingUtilities.invokeLater( new Runnable()
{
public void run()
{
JFrame frame = new JFrame();

final DefaultListModel listModel = new DefaultListModel();
Object[] test = { "One", "Two", "Three" };
for( Object li : test )
listModel.addElement( li );

final JList list = new JList( listModel );
frame.add( list );

Box vbox = Box.createVerticalBox();
JButton addButton = new JButton( "Add" );
vbox.add( addButton );
JButton removeButton = new JButton( "Remove" );
vbox.add( removeButton );
frame.add( vbox, BorderLayout.EAST );

addButton.addActionListener( new ActionListener()
{
int count = 0;
@Override
public void actionPerformed( ActionEvent e )
{
listModel.addElement( "Add "+ ++count );
list.setSelectedIndex( listModel.getSize()-1 );
}
} );

frame.pack();
frame.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE );
frame.setSize( 350, 350 );
frame.setLocationRelativeTo( null );
frame.setVisible( true );
}
} );
}
}
 
M

markspace

I'd have said not -- based on output of the even-more-debug-prints
version I'm reasonably confident that I'm not getting that kind of
recursion/loop, and anyway I only modify the selection in code to
process add/remove events, rather than in ....


I didn't read your code carefully to find the bug -- there was quite a
lot of it. Rather I just focused on your idea that there might be a bug
in JList or DefaultModelListener. So I purposely started over from
scratch, since that should duplicate the bug if there was one.

When it worked, I basically stopped there and posted the code I had. No
sense in belaboring something that's working.

Anyway, glad you got yourself straightened out. Good luck.
 
B

blmblm

I didn't read your code carefully to find the bug -- there was quite a
lot of it.

Yes. Sorry about that. I was trying for an SSCCE but the "short"
part eluded me.
Rather I just focused on your idea that there might be a bug
in JList or DefaultModelListener. So I purposely started over from
scratch, since that should duplicate the bug if there was one.

Yeah, maybe .... Well, or at least it would show whether what I
had in mind was possible. My thinking is that the library code
shouldn't be setting a selection index outside the bounds of the
underlying model, but maybe it doesn't count as a bug if it only
happens when the user (me) does something wrongheaded.
When it worked, I basically stopped there and posted the code I had. No
sense in belaboring something that's working.
:)

Anyway, glad you got yourself straightened out. Good luck.

Yes. Thanks again. Like I said, the big difference between what
you did (which works) and what I was doing (which doesn't) is that
you put the code to set the selection in with the code to change
the data model, which -- well, the more I think about it the more
I wonder why I didn't do it that way myself.

For the record-or-whatever, my revised (and working) code is below.
I've folded this change back into my "real" code [*] (which is
itself a prototype for something else), and now it works too. Whew!

[*] Which is not actually Java code but Scala. But there apparently
isn't a Scala Usenet group (only a Google group), and anyway it
seemed like it might be useful to know whether the problem was with
Scala's wrappers around Swing or could be reproduced in straight
Java, which in this case it could.


==== start of code ====

/*
Attempt at an editable JList, supporting allow adding, removing, and
changing elements.
*/

import java.awt.*;
import java.awt.event.*;

import javax.swing.*;
import javax.swing.event.*;


public class TryListMain extends JFrame {

// ---- variables ----

private static boolean DEBUG = true;

private DefaultListModel model;
private TryList list;

private static int counter = 0;

// ---- constructor ----

public TryListMain() {
super();
model = new DefaultListModel();
list = new TryList(model);
setLayout(new BorderLayout());
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
add(new ButtonPanel(), BorderLayout.NORTH);
add(list, BorderLayout.CENTER);
}

// ---- methods ----

public TryList getList() { return list; }
public DefaultListModel getModel() { return model; }

public static void debugPrint(String msg) {
if (DEBUG) {
if (msg.length() > 0) {
System.out.println("DBG "+msg);
} else {
System.out.println();
}
}
}
public static void debugPrint() {
debugPrint("");
}

private String generateItem() {
++counter;
return "Element "+counter;
}

// ---- main ----

public static void main(String[] args) {

SwingUtilities.invokeLater(new Runnable() {
@Override
public void run() {

UIManager.put("Button.defaultButtonFollowsFocus", true);
TryListMain theFrame = new TryListMain();

System.out.println("data listeners:");
for (ListDataListener ll :
theFrame.getModel().getListDataListeners())
{
System.out.println(ll.getClass().getName());
}
System.out.println();

System.out.println("list selection listeners:");
for (ListSelectionListener ll :
theFrame.getList().getListSelectionListeners())
{
System.out.println(ll.getClass().getName());
}
System.out.println();

theFrame.setSize(new Dimension(400,200));
theFrame.setVisible(true);
}
});
}

// ---- classes ----

private class ButtonPanel extends JPanel {

private Action removeAction;
private Action replaceAction;
private Action newAction;
private Action showAction;

private void setActionsEnabled() {
int si = list.selectedIndex();
removeAction.setEnabled(si >= 0);
replaceAction.setEnabled(si >= 0);
}

public ButtonPanel() {

super(new FlowLayout());

removeAction = new AbstractAction("Remove") {
@Override
public void actionPerformed(ActionEvent e) {
int si = list.selectedIndex();
TryListMain.debugPrint();
TryListMain.debugPrint("== remove item at "+si);
if (si >= 0) {
model.remove(si);
if (model.getSize() == 0) {
list.clearSelection();
} else {
int newSelect = Math.max(0, si-1);
list.setSelectedIndex(newSelect);
list.ensureIndexIsVisible(newSelect);
}
} else {
// FIXME should never happen
System.err.println("no selected item to remove");
}
}
};

replaceAction = new AbstractAction("Replace") {
@Override
public void actionPerformed(ActionEvent e) {
int si = list.selectedIndex();
if (si >= 0) {
String item = generateItem();
TryListMain.debugPrint();
TryListMain.debugPrint("== replace item at "+si);
model.set(si, item);
} else {
// FIXME should never happen
System.err.println("no selected item to replace");
}
}
};

newAction = new AbstractAction("New") {
@Override
public void actionPerformed(ActionEvent e) {
String item = generateItem();
TryListMain.debugPrint();
TryListMain.debugPrint("== add new item");
model.addElement(item);
int newSelect = model.getSize()-1;
list.setSelectedIndex(newSelect);
list.ensureIndexIsVisible(newSelect);
}
};

showAction = new AbstractAction("Show selected") {
@Override
public void actionPerformed(ActionEvent e) {
int si = list.selectedIndex();
if (si < 0) {
System.out.println("No selected value");
} else {
System.out.println(
String.format("Selected value (%d of %d) '%s'",
si+1, model.getSize(), model.get(si)));
}
}
};

list.addListSelectionListener(new ListSelectionListener() {
@Override
public void valueChanged(ListSelectionEvent e) {
setActionsEnabled();
}
});

setActionsEnabled();

add(new JButton(removeAction));
add(new JButton(replaceAction));
add(new JButton(newAction));
add(new JButton(showAction));
}
}
}

class TryList extends JList {

// ---- constructor ----

public TryList(DefaultListModel lModel) {
super(lModel);

getSelectionModel().setSelectionMode(
ListSelectionModel.SINGLE_SELECTION);

addListSelectionListener(new ListSelectionListener() {
@Override
public void valueChanged(ListSelectionEvent e) {
TryListMain.debugPrint(String.format(
"selection changed to %s, source %s",
selectedIndicesToString(),
e.getSource().getClass().getName()));
}
});
}

// ---- methods ----

// return -1 if nothing selected, or out of range
public int selectedIndex() {
int[] si = getSelectedIndices();
if ((si.length == 1) && (si[0] < getModel().getSize())) {
return si[0];
} else {
return -1;
}
}

private String selectedIndicesToString() {
StringBuffer sb = new StringBuffer();
for (int i : getSelectedIndices()) {
if (sb.length() > 0) sb.append(' ');
sb.append(i);
}
return "("+sb.toString()+")";
}
}
 
B

blmblm

Also, 'DEBUG' does not follow the Java naming conventions.

Wouldn't this comment make more sense if you quoted the single
line that declares the variable in question? my first thought was
"and why not? isn't the relevent convention that constants have
all-caps names?"

But then if I look at the code, um, well, that may have been my
intent, but it's not what I wrote, is it? I should have also
have declared it final, yes? Well, my only defense (such as it is)
is that lately I've been writing a lot more Scala than Java, and
it's surprising (to me anyway) how quickly one adopts new habits.
 
L

Lew

blmblm said:
Wouldn't this comment make more sense if you quoted the single
line that declares the variable in question? my first thought was

I'm sorry, it's your code. You didn't know to look where you declared the variable in question?

Where else would you look, then?

No, it wouldn't make more sense if I quoted the code that you already have in front of you.
I expect you to be able to follow that elementary trail without difficulty. Especially in your case,
as you have a history in this newsgroup of knowing what you're about.
"and why not? isn't the relevent [sic] convention that constants have
all-caps names?"

But then if I look at the code, um, well, that may have been my
intent, but it's not what I wrote, is it? I should have also
have declared it final, yes? Well, my only defense (such as it is)

'final' by itself is not enough to make something a constant, or even immutable.

A 'final' reference to a mutable object should be spelled not with all-caps but in the
normal camel case.
is that lately I've been writing a lot more Scala than Java, and
it's surprising (to me anyway) how quickly one adopts new habits.

The Java Coding Conventions suggest spelling constant variables in all caps.
I extend that to 'final' references to immutable objects, even though they are not
semantically equivalent to constant variables. Most people do this. However it is
misleading when they go a step further and spell all 'final' variables with all caps.
 
B

blmblm

I'm sorry, it's your code. You didn't know to look where you declared the
variable in question?

*I* knew. What about other readers? C'mon, you're the stickler for
following conventions, and isn't it a convention that Usenet posts
should be, as much as feasible, self-contained?

(I begin to believe that the people who chide you for being overly
argumentative have a point. But then there's the bit of flattery
later. Hm.)

Where else would you look, then?

No, it wouldn't make more sense if I quoted the code that you already
have in front of you. I expect you to be able to follow that
elementary trail without difficulty.

*But I don't* (have it in front of me): If you check headers, you'll
notice that I'm using the very old trn, which as far as I know does
not provide any easy way to display two posts at the same time,
and while I do usually have saved copies of previous posts in some
other window-or-the-like .... Well, no, no one needs to hear details
of my old-school-to-a-fault tools setup. The point is that you
could save all readers some trouble by quoting *one line*.

Especially in your case,
as you have a history in this newsgroup of knowing what you're about.

Flattery will .... something.

"and why not? isn't the relevent [sic] convention that constants have
all-caps names?"

Good (if irritating) catch on the spelling mistake. I know better.

'final' by itself is not enough to make something a constant, or even
immutable.

A 'final' reference to a mutable object should be spelled not with
all-caps but in the normal camel case.

The variable I named "DEBUG" is a boolean, so I don't quite get how a
convention for naming references to mutable objects applies.


(I don't really know why I brought that up, since all-caps variable
names are not idiomatic in Scala. Just sayin', maybe.)

The Java Coding Conventions suggest spelling constant variables in all caps.
I extend that to 'final' references to immutable objects, even though they are not
semantically equivalent to constant variables. Most people do this. However it is
misleading when they go a step further and spell all 'final' variables with all caps.

Okay. How is that relevant ....
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top