JDK-4629242 : Need to specify behavior for Container.remove(int) for an invalid index
  • Type: Bug
  • Component: client-libs
  • Sub-Component: java.awt
  • Affected Version: 1.0-b21,1.0-b22
  • Priority: P3
  • Status: Closed
  • Resolution: Duplicate
  • OS: linux,solaris_7
  • CPU: generic,sparc
  • Submitted: 2002-01-28
  • Updated: 2006-08-07
  • Resolved: 2006-08-07
Related Reports
Duplicate :  
Relates :  
Description
======== This is a new description - see below for the old one =========

It's unclear what should happen when removing a component at an invalid index.
It used to be inconsistent before: JDK 1.4.0 used to throw NullPointerException 
in some cases and ArrayIndexOutOfBoundsException in others.

Bug 4546535 was fixed in J2SE to always throw ArrayIndexOutOfBoundsException
(I think). Unfortunately, the same bug was fixed to throw IllegalArgumentException
in J2ME PP (see the suggested fix - it was my proposal, sorry). It would be
good to resolve this inconsistency.

###@###.### 2004-10-01

================= This is the old description ==========================

Name: atR10191			Date: 01/28/2002


(it's java bug 4546535 filed against ri in accordance with agreement
with ri team)

This has been initially reported in 1997 against 1.1 as a bug 4026541.
The bug 4026541 has been closed as not a bug since the example provided no
longer exhibits the problem. The problem is that Container.remove(int)
method can throw different exception under certain conditions. I've found
that it always throw ArrayIndexOutOfBoundsException except the case then
the Component has been already added/removed in the past. In the latter
case NPE is thrown and this is the unexpected behavior. The following
example can be used to demonstrate the bug still exist in 1.4:

----------------- TesX.java ---------------------
import java.awt.*;
import java.util.*;

public class TesX {
    public static void main( String argv[] ) {
        Container p=new Container();
        p.add(new Container());
        p.remove(0);

        int[] bad = {-1, 0, 1};
        for (int i=0; i<bad.length; i++)
        try {
           System.err.println("Removing "+bad[i]);
           p.remove(bad[i]);
           System.err.println("No exception");
        } catch(Exception e) {
           System.err.println(e);
        }
    }
}
-------------------------------------------------

An output under pbp-b22/pp-b42 will be:

----------- example output ---------

Removing -1
java.lang.ArrayIndexOutOfBoundsException
Removing 0
java.lang.NullPointerException
Removing 1
java.lang.NullPointerException
------------------------------------

So the NPE is thrown for a position where a component was added and then
removed in the past.
======================================================================

The addition/removal of the new Container does not affect the behaviour of the test. 

When the Container is instantiated, the default constructor creates a Container with a default array length of 4 null components.

Hence, attempts to remove a component from the Container at index < 0 or > 3 will result in an ArrayIndexOutOfBoundsException. 

Attempts to remove a component from the Container at index >= 0 or <= 3, will result in a NullPointerException if no component exists within that index of the component array.

However, the exception thrown (if any) should be consistent across all implementations. Hence, it is not acceptable for the NullPointerException to be thrown as the RI has a default creation of an empty 4-element component array. Another implementation might create an empty 5-element array that would result in different behaviour with a different exception thrown.

Hence, a suggested check for the remove(int index) method should:

1. Check if the Container actually contains any components before attempting to remove it.
2. If so, check that the index references an element of the array that actually contains a component.

I would suggest that the spec specify that an ArrayOutOfBoundsException should be thrown if neither criteria are met.

###@###.### 2002-01-29

======================================================================
Changing category to specification so as to determine what exception should be thrown in this case - if any.

It seems that the specification should define the behaviour of a call to Container.remove(int index) with an invalid index.

At present, the specification does not define this behaviour - indicating that the method should fail silently. Would it be more helpful to the user to have this behaviour defined in order to indicate where the error has occurred? 

With that in mind, should the specification document that this method can result in an unchecked runtime exception in the @throws tag of this method definition when called with an invalid index - eg. IndexOutOfBoundsException.

Note - this is also an issue for the Personal Profile Spec.

###@###.### 2002-02-20

================================================================================

Comments
EVALUATION Closing as duplicate of 4647973 which was fixed in mustang b95
07-08-2006

EVALUATION The default Container constructor creates an array of 4 null components. Hence, attempts to remove a component from the Container at index < 0 or > 3 will result in an ArrayIndexOutOfBoundsException. Attempts to remove a component from the Container at index >= 0 or <= 3, will result in a NullPointerException if no component exists within that index element of the component array. The behaviour of the test is the same whether the container has had a component removed or not. Should the spec include a note regarding the default creation of the component array of length 4 which explains the occurence of the NullPointerException? ###@###.### 2002-01-28 ====================================================================== The exception thrown should be consistent across all implementations. Hence, it is not acceptable for the NullPointerException to be thrown as the RI has a default creation of an empty 4-element component array. Another implementation might create an empty 5-element array that would result in different behaviour with a different exception thrown. The remove method needs to check if the container contains any components and then if the index references an element that contains a component. Suggest that the specification specify that an ArrayOutOfBoundsException should be thrown if neither criteria are met. ###@###.### 2002-01-29 ====================================================================== According to the spec this method does not throw any exceptions and should fail silently in case of wrong parameters. ###@###.### 2002-02-04 ====================================================================== Changed category to specification for clarification on behaviour when invalid index is passed as parameter to the remove method. ###@###.### 2002-02-20 ====================================================================== This looks like a JDK bug. Submitter should file a bug against the J2SE spec and reference it here. RI should follow behavior of JDK 1.3.1. ###@###.### 2002-03-05 Yes, the J2SE spec bug will be filed. However, I don't think the RI should follow behavior of JDK 1.3.1: it is obviously broken. We should be at least consistent in throwing the same type of exception when the index is invalid. ###@###.### 2002-03-05 ====================================================================== IllegalArgumentException is now thrown when invalid index is called (b29). ###@###.### 2002-03-06 ======================================================================
06-03-2002

SUGGESTED FIX The exception thrown should be consistent across all implementations. Hence, the remove(int index) method should: 1. Check if the Container actually contains any components before attempting to remove it. 2. If so, check that the index references an element of the array that actually contains a component. Suggest that specification specifiy that an ArrayOutOfBoundsException should be thrown if neither criteria are met. The diffs are: ------- Container.java ------- 335a336,341 > /* 4629242 - Check if Container contains any components or if > * index references a null array element. > */ > if (ncomponents == 0 || component[index] == null){ > throw new ArrayIndexOutOfBoundsException("Invalid index : " + index + "."); > } ###@###.### 2002-01-29 ====================================================================== I think that IllegalArgumentException may be a better choice, since it doesn't imply that the internal representation of a list of components is an array. ###@###.### 2002-03-05 ====================================================================== IllegalArgumentException is now thrown when invalid index is called (b29). ###@###.### 2002-03-06 ======================================================================
06-03-2002