Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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.

like image 399
nachokk Avatar asked Jan 10 '14 19:01

nachokk


2 Answers

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.

like image 171
kleopatra Avatar answered Oct 01 '22 22:10

kleopatra


Generally speaking, there is no easy (or right) answer to this question.

Does overriding getPreferredSize break Liskov Substitution Principle? Yes (based on the available documentation).

But doesn't most Object extension? What would be the point of changing the behaviour of a method if it had to adhere strictly to the expectations of the original implementation (yes, there are good examples when you should do this, like hashcode and equals and others where the line is grayed)?

In this case, the problem seems to extend from the inappropriate use of setXxxSize and that fact that these methods are actually public. Why are they public? I have no idea, as they are the cause of more problems than just about any other part of the API (including KeyListener).

Overriding getPreferredSize is preferred as the change is carried with the object, unlike calling setPreferredSize from outside the object ownership/context

Because getXxxSize is suppose to provide sizing hints to the layout manager, there doesn't actually seem to be any reasonably good reason to actually have the setXxxSize methods public as, IMHO, developers shouldn't be messing with them - a component is required to provided the best estimation of the size it needs based on it's own internal requirements.

The reason for overriding getXxxSize in this manner would also be to prevent other people from changing the value you've specified, which could be made for particular reasons.

On one hand, as you've suggested, we have an expectation of the API, but on the other hand, there are times when we want to control the size and lots of times when you don't want the user to change the value.

My personal feeling is to ignore setXxxSize as much as possible (or treat it as protected). One of the reasons for overriding getXxxSize is stop people from changing the size, but equally, you could override setXxxSize and throw a not supported exception.

If you were to document the decisions for ignoring setXxxSize would that constitute a break of Liskov Substitution Principle? Possibly, as the component can still act like it's parent.

My general gut feeling is to understand what Liskov Substitution Principle is trying to do, know when you should use it and when you shouldn't. There can't be clear cut rule that meets every case, especially when you consider a case where the design itself is wrong.

Based on your example, you shouldn't be overriding getXxxSize or setXxxSize at all, but call setXxxSize from the constructor, as this would maintain the current API contract, but would also step on the toes of calling a overridable methods from the constructor...

So everywhere you look, you're stepping on someone's toes...

The short of it all. If it's important to you (to maintain Liskov Substitution Principle), you should use setXxxSize from within your own components context. The problem with this is that it's impossible to stop someone from wiping out your design decisions with there own values and, as I stated in the comments, when people do this without actually understanding what they are doing, this just makes everybody elses job a nightmare.

Don't abuse setPreferredSize, use it only from within the context of the object instance and resist calling it from outside...IMHO

like image 23
MadProgrammer Avatar answered Oct 01 '22 23:10

MadProgrammer