Name: jk109818 Date: 09/23/2002


FULL PRODUCT VERSION :
java version "1.4.1"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.1-b21)
Java HotSpot(TM) Client VM (build 1.4.1-b21, mixed mode)


FULL OPERATING SYSTEM VERSION :
Microsoft Windows 2000 [Version 5.00.2195]

ADDITIONAL OPERATING SYSTEMS :
not platform specific: I'm looking at the source code.



A DESCRIPTION OF THE PROBLEM :
As of 1.4.0, AbstractButton implements
configurePropertiesFromAction() like this:

    protected void configurePropertiesFromAction(Action a) {
        configurePropertiesFromAction(a, null);
    }

    void configurePropertiesFromAction(Action a, String[]
types) {
        if (types == null) {
            String[] alltypes = { Action.MNEMONIC_KEY,
Action.NAME,
                                  Action.SHORT_DESCRIPTION,
Action.SMALL_ICON,
                                  Action.ACTION_COMMAND_KEY,
"enabled" };
            types = alltypes;
        }
        for (int i=0; i<types.length; i++) {
            String type = types[i];
            if (type == null) continue;

            if (type.equals(Action.MNEMONIC_KEY)) {
                Integer n = (a==null) ? null :
Integer)a.getValue(type);
                setMnemonic(n==null ? '\0' : n.intValue());
            } else if /* and so on for the others */

Then subclasses hook into this like this (notes in
comments are mine, not actually in code):

    /* JButton's implementation */
    protected void configurePropertiesFromAction(Action a) {
        String[] types = { Action.MNEMONIC_KEY,
Action.SHORT_DESCRIPTION,
                           Action.SMALL_ICON,
Action.ACTION_COMMAND_KEY,
                           "enabled" };
    /* This list is just like AbstractButton's except
Action.NAME has been removed. */
        configurePropertiesFromAction(a, types);

Boolean hide = (Boolean)getClientProperty("hideActionText");
    /* now handle Action.NAME specially */
setText((( a != null && (hide == null || hide!=Boolean.TRUE))
? (String)a.getValue(Action.NAME)
: null));
    }

    /* JCheckBox's implementation */
    protected void configurePropertiesFromAction(Action a) {
        String[] types = { Action.MNEMONIC_KEY, Action.NAME,
                           Action.SHORT_DESCRIPTION,
                           Action.ACTION_COMMAND_KEY,
"enabled" };
    /* This list is just like AbstractButton's except
Action.SMALL_ICON has been removed. */
        configurePropertiesFromAction(a, types);
    }

That's only two of the subclasses, but you get the idea.

I'm not real happy with this scheme, because:
(1) These String[] arrays are getting instatiated on
    every call, even though they are the same on every
    call. Shoudn't they be shared (static) within the
    class? (As someone who was once trying to get a
    Swing app runing on a winCE device but had to give
    up becasue it was too slow, I do worry about stuff
    like this within the Swing package.)

(2) Performing a linear-time if/elseif/elseif/elseif/etc.
    search on every element of the array seems wasteful,
    but I can't think of anything better. Since the length
    of the array tops out at about 7 evelements, I guess
    this ok in practice? n*(n+1)/2 comparisons instead
    of n, is only 21 extra comparisons if n==7

(3) The subclasses of AbstractButton want to be able to
    say "all the properties except this one," but can't
    do that with a String[] array. As a result, every
    subclass has to specify all the properties explitly,
    which is asking for trouble.

    Consider the scenario where we want to add a new
    property that will be respected by all the subclasses
    of AbstractButton. (This is not unlikely. Take a look
    at the RFE with internal review ID 164827, which points
    out that adding the setDisplayedMnemonicIndex() method
    to AbstractButton in 1.4 means it should also have been
    supported by Action and cofigurePropertiesFromAction().)

    We would hope that all we would have to do is add support
    for displayedMnemonicIndex to AbstractButton and all its
    subclasses would inherit the new functionality. That
    would be the case if the subclasses were actually able
    to say "all properties except Action.NAME", but they
    don't, so supporting displayedMnemonicIndex will require
    changing the code of every subclass.

It seems to me that since
  configurePropertiesFromAction(Action, String[])
is package-private, it could be replaced with
something else without breaking anyone's code.

Perhaps use a java.util.HashSet instead of a String[]
array, because it supports O(1) lookup and remove().

Code in JButton could look like this:

  /* shadow the static field 'types' to remove Action.NAME */
  static HashSet types =
((java.util.Set)AbstractButton.types.clone()).remove(Action.NAME);

Please do consider it. I'd really like to be able to
use Actions more than I can now. They have such potential.


REPRODUCIBILITY :
This bug can be reproduced always.
(Review ID: 164839)
======================================================================