On Thu, 2006-07-27 at 17:05 -0400, Anthony Balkissoon wrote:
> A couple of things to check first:
>
> - getSelectedIndex() should probably return -1 if no items are selected,
> not 0.
Correct, it should return -1, the patch that Tania submitted did return
-1, the branch "if (selected == null) return 0" was never taken because
selected was initialized in the constructor and not set to null
anywhere. It could potentially be null if the peer returns null for
getSelectedIndexes() and in this case -1 should still be returned so I
removed that if statement.
> - in delItem, if the last item is deleted, but it was selected, it looks
> like an ArrayIndexOutOfBoundsException will be thrown
Actually, no. If there are 3 items and the third one (index 2) is
selected, and then delItem(2) is called, isSelected(2) still returns
true. This doesn't throw any exceptions. getItem(2) will throw an
exception but isSelected(2) should still return true. I augmented the
Mauve tests to show this.
> - what happens in select() if multiple mode is false and another index
> is already selected? should be call deselect() here?
The way it is now seems to be fine, initializing a new int array.
All of these things should also be tested when there IS a peer present.
For instance, in select(), if a peer is present, we call peer.select().
I wonder if we should return at this point or continue on with the
method. These tests need to be written.
The patch attached is essentially Tania's patch, with the aforementioned
removal of an if statement and also with one small performance tweak in
the select() method when multipleMode is false.
I'll leave this for Tania to comment on and commit.
--Tony
Index: java/awt/List.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/List.java,v
retrieving revision 1.28
diff -u -r1.28 List.java
--- java/awt/List.java 13 Jul 2006 17:30:24 -0000 1.28
+++ java/awt/List.java 27 Jul 2006 21:46:45 -0000
@@ -108,7 +108,7 @@
* @serial An index value used by <code>makeVisible()</code> and
* <code>getVisibleIndex</code>.
*/
-private int visibleIndex;
+private int visibleIndex = -1;
// The list of ItemListeners for this object.
private ItemListener item_listeners;
@@ -116,7 +116,6 @@
// The list of ActionListeners for this object.
private ActionListener action_listeners;
-
/*************************************************************************/
/*
@@ -176,6 +175,7 @@
if (GraphicsEnvironment.isHeadless())
throw new HeadlessException ();
+
}
/*************************************************************************/
@@ -314,12 +314,13 @@
*/
public void
setMultipleSelections(boolean multipleMode)
-{
+{
this.multipleMode = multipleMode;
ListPeer peer = (ListPeer) getPeer ();
if (peer != null)
peer.setMultipleMode (multipleMode);
+
}
/*************************************************************************/
@@ -519,6 +520,12 @@
public void
addItem(String item, int index)
{
+ if (item == null)
+ item = "";
+
+ if (index < -1)
+ index = -1;
+
if ((index == -1) || (index >= items.size ()))
items.addElement (item);
else
@@ -543,7 +550,17 @@
public void
delItem(int index) throws IllegalArgumentException
{
+ boolean selected = false;
+ if (isSelected(index))
+ {
+ selected = true;
+ deselect(index);
+ }
+
items.removeElementAt (index);
+
+ if (selected)
+ select(index);
ListPeer peer = (ListPeer) getPeer ();
if (peer != null)
@@ -580,15 +597,6 @@
public synchronized void
delItems(int start, int end) throws IllegalArgumentException
{
- if ((start < 0) || (start >= items.size()))
- throw new IllegalArgumentException("Bad list start index value: " + start);
-
- if ((start < 0) || (start >= items.size()))
- throw new IllegalArgumentException("Bad list start index value: " + start);
-
- if (start > end)
- throw new IllegalArgumentException("Start is greater than end!");
-
// We must run the loop in reverse direction.
for (int i = end; i >= start; --i)
items.removeElementAt (i);
@@ -644,6 +652,8 @@
ListPeer peer = (ListPeer) getPeer ();
if (peer != null)
peer.removeAll ();
+
+ selected = new int[0];
}
/*************************************************************************/
@@ -695,6 +705,7 @@
if (selected == null || selected.length != 1)
return -1;
+
return selected[0];
}
@@ -713,7 +724,8 @@
{
ListPeer l = (ListPeer) peer;
selected = l.getSelectedIndexes ();
- }
+ }
+
return selected;
}
@@ -862,13 +874,32 @@
*
* @param index The index of the item to select.
*/
-public synchronized void
-select(int index)
-{
- ListPeer lp = (ListPeer)getPeer();
- if (lp != null)
- lp.select(index);
-}
+ public synchronized void select(int index)
+ {
+ ListPeer lp = (ListPeer) getPeer();
+ if (lp != null)
+ lp.select(index);
+
+ boolean found = false;
+ for (int i = 0; i < selected.length; i++)
+ {
+ if (selected[i] == index)
+ found = true;
+ }
+ if (! found)
+ {
+ if (! isMultipleMode())
+ {
+ selected = new int[] { index };
+ return;
+ }
+
+ int[] temp = new int[selected.length + 1];
+ System.arraycopy(selected, 0, temp, 0, selected.length);
+ temp[selected.length] = index;
+ selected = temp;
+ }
+ }
/*************************************************************************/
@@ -880,9 +911,26 @@
public synchronized void
deselect(int index)
{
- ListPeer lp = (ListPeer)getPeer();
- if (lp != null)
- lp.deselect(index);
+ if (isSelected(index))
+ {
+ ListPeer lp = (ListPeer)getPeer();
+ if (lp != null)
+ lp.deselect(index);
+
+ int[] temp = new int[selected.length - 1];
+ for (int i = 0; i < temp.length; i++)
+ {
+ if (selected[i] != index)
+ temp[i] = selected[i];
+ else
+ {
+ System.arraycopy(selected, i + 1, temp, i,
+ selected.length - i - 1);
+ break;
+ }
+ }
+ selected = temp;
+ }
}
/*************************************************************************/
