Search code examples
javaswingjcomponentliskov-substitution-principle

Overriding getPreferredSize() breaks LSP


I always see advices in this site of overriding getPreferredSize() instead of using setPreferredSize() as shown in these previous threads for example.

  1. Use of overriding getPreferredSize() instead of using setPreferredSize() for fixed size Components
  2. Should I avoid the use of set(Preferred|Maximum|Minimum)Size methods in Java Swing?
  3. Overriding setPreferredSize() and getPreferredSize()

See this example:

public class MyPanel extends JPanel{

  private final Dimension dim = new Dimension(500,500); 

  @Override
  public Dimension getPreferredSize(){
      return new Dimension(dim);
  }

 public static void main(String args[]){
      JComponent component = new MyPanel();
      component.setPreferredSize(new Dimension(400,400));
      System.out.println(component.getPreferredSize());
 }

}

setPreferredSize()

  • Sets the preferred size of this component.

getPreferredSize()

  • If the preferredSize has been set to a non-null value just returns it. If the UI delegate's getPreferredSize method returns a non null value then return that; otherwise defer to the component's layout manager.

So doing this clearly breaks Liskov Substitution Principle.

prefferedSize is a bound property so when you set it a firePropertyChange is executed. So my question is when you override getPrefferedSize() don't you need to override setPreferredSize(..) too?

Example:

 public class MyPanel extends JPanel{

  private Dimension dim = null; 

  @Override
  public Dimension getPreferredSize(){
      if(dim == null)
       return super.getPreferredSize();
      return new Dimension(dim);
  }

  @Override
  public void setPrefferedSize(Dimension dimension){
        if(dim == null)
            dim = new Dimension(500,500);
        super.setPreferredSize(this.dim); //
  }

 public static void main(String args[]){
      JComponent component = new MyPanel();
      component.setPreferredSize(new Dimension(400,400));
      System.out.println(component.getPreferredSize());
 }

}

Now we see that we get identical results but listeners will get notified with real values and besides we don't break LSP cause setPreferredSize states Sets the preferred size of this component. but not how.


Solution

  • Several aspects to this interesting question (Mad already mentioned the spare-my-fellow-developer)

    Do we violate the LSP in overriding only getXXSize() (vs. setXXSize() as well)?

    Not if we do it correctly :-) First authority is the API doc of the property, best from its origin, that is Component:

    Sets the preferred size of this component to a constant value. Subsequent calls to getPreferredSize will always return this value.

    This is a binding contract, so however we implement the getter it has to respect the constant value if set:

    @Override
    public Dimension getPreferredSize() {
        // comply to contract if set
        if(isPreferredSizeSet())
            return super.getPreferredSize();
        // do whatever we want
        return new Dimension(dim);
    }
    

    XXSize is a bound property - is it?

    In JComponent's ancestry there is circumstantial evidence only: actually, Component fires a PropertyChangeEvent in the setter. JComponent itself seems to document the fact (bolding by me):

    @beaninfo preferred: true bound: true description: The preferred size of the component.

    Which is ... plain wrong: being a bound property implies that listeners need to be notified whenever the value changes, that is the following (pseudo-test) must pass:

    JLabel label = new JLabel("small");
    Dimension d = label.getPreferredSize();
    PropertyChangeListener l = new PropertyChangeListener() ...
        boolean called;
        propertyChanged(...) 
            called = true;
    label.addPropertyChangeListener("preferredSize", l);
    label.setText("just some longer text");
    if (!d.equals(label.getPreferredSize())
       assertTrue("listener must have been notified", l.called); 
    

    ... but fails. For some reason (no idea why that might have deemed appropriate) they wanted the constant part of xxSize to be a bound property - such overlays are simply not possible. Could have been (wildly guessing, of course) a historic issue: initially, the setter was available in Swing only (for good reasons). In its backport to awt it mutated into a bean property that it never was.