Name: yyT116575 Date: 03/14/2001


java version "1.3.0"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.3.0-C)
Java HotSpot(TM) Client VM (build 1.3.0-C, mixed mode)

The method:
public void fireVetoableChange(PropertyChangeEvent evt)
   throws PropertyVetoException;
in the class java.beans.VetoableChangeSupport behaves (IMO) incorrectly.

A PropertyChangeEvent is fired to VetoableChangeListeners to request a pending
property change, not to indicate that the change has actually occured.

When a Listener vetoes the property change, the above method creates a new
event, with the oldValue and newValue fields swapped, and fires that off to all
listeners, and then ignores any vetoes of this second propertychangeevent. I
shall call this second propertychangeevent an "undo" event.

This seems to be incorrect behaviour, as since the property change is pending
(has not yet occured), it does not need 'undoing'. If one wishes to confirm
that a change occured after accepting a Vetoable PropertyChangeEvent, one
should also implement the PropertyChangeListener interface.

Additionally, there is no way for a VetoableChangeListener to tell the
difference between an "undo" PropertyChangeEvent and a regular
PropertyChangeEvent. This can give a PropertyChangeListener the view that it
has just vetoed 2 subsequent changes, when it really vetoed a change, and then
vetoed its "undo" event.

I believe that this "undo" event should never be sent.

My suggestion for the implementation of this method is:

  /**
   * Fire a vetoable property update to any registered listeners. If
   * anyone vetos the change, then fire a new event reverting everyone to
   * the old value and then rethrow the PropertyVetoException.
   * <p>
   * No event is fired if old and new are equal and non-null.
   *
   * @param evt The PropertyChangeEvent to be fired.
   * @exception PropertyVetoException if the recipient wishes the property
   * change to be rolled back.
   */
  public void fireVetoableChange(PropertyChangeEvent evt)
      throws PropertyVetoException {

    Object oldValue = evt.getOldValue();
    Object newValue = evt.getNewValue();
    String propertyName = evt.getPropertyName();
    if (oldValue != null && newValue != null && oldValue.equals(newValue)) {
      return;
    }

    java.util.Vector targets = null;
    VetoableChangeSupport child = null;
    synchronized (this) {
      if (listeners != null) {
        targets = (java.util.Vector) listeners.clone();
      }
      if (children != null && propertyName != null) {
        child = (VetoableChangeSupport)children.get(propertyName);
      }
    }

    if (listeners != null) {
      for (int i = 0; i < targets.size(); i++) {
        VetoableChangeListener target = (VetoableChangeListener)targets.elementAt(i);
        target.vetoableChange(evt);
      }

      if (child != null) {
        child.fireVetoableChange(evt);
      }
    }
  }
(Review ID: 118845)
======================================================================