Overriding getPreferredSize() breaks LSP

Question

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.

Confrere

I don't see it as silly. 1+ and awaiting kleopatra's response. – Tallyman

That's an interesting question. The issues arises from the choice of the designers to expose setPreferredSize instead of actually forcing people to override getPreferredSize. If you look at just about UI component, there doesn't seem to be any logical reason to actually use setPreferredSize (that won't actually break the component). The reason for overriding getPreferredSize in this manner would also be to prevent other people from changing the value you've specified, which could made for particular reason. If you want to, you could call super.getPreferredSize and check if its null or not – Archon

As a side note, the previous developer on our project used setPreferredSize on just about everything he touched. It completely broke the application when we shifted from Win XP to, well anything else, so, setPreferredSize = pain in ass IMHO – Archon

@HovercraftFullOfEels i asked her somewhere but i can't find it! – Confrere

I just noticed that you are passing back the same instance of Dimension from your getPreferredSize method, this is very dangerous, as Dimension is mutable by the caller, that is, they can change the values. This will effect the values returned on subsequent calls, meaning not only have I being able to circumvent setPreferredSize, but have broken your design – Archon

@Archon you don't see edit xD i edited before you comment – Confrere

@Confrere I was using a cached page ;) – Archon

Wouldn't it be better not to override setPreferredSizea and simply check super.getPreferrdSize, if it's null, return your Dimension, otherwise return the super value? This would reduce the complexity slightly - IHMO – Archon

@Archon yeah i think that is the answer i was hoping (i must to say that i've already ask this to kleopatra an answer something like you said) but im not a swing pro as you, so i want to know why we override getPreferredSize if it's only when a null it's returned. You sure can make this a complete answer :D – Confrere

but you forgot for one important think, JComponents can returns any real size after pack() is called or if is already visible, this is how Layout Managers works (excluding NullLayout by using Insets) then everyhting here can/could be/is about theory only, academic discusion – Tereasaterebene

It's a very complicate issue. 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 :P - My personal feeling is to ignore setPreferredSize as much as possible (or treat it as protected). One of the reasons for overriding getPreferredSize is stop people from changing the size, but equally, you could override setPreferredSize and throw a not supported exception :P – Archon

@Tereasaterebene that's truth, perhaps is just an academical question, but perhaps is there a good alternative to don't break lsp perhaps making our inmutableSizeComponent and throwing not supporting exception – Confrere

there is no ambiguity/problem: the bound property is the constant pref (or min/max) as configured in the setXXSize, not the calculated. Documentation is a bit ... incomplete, though, strictly speaking not existing because only fully explained as a code comment. +1 for the question, of course :-) – Sisto

@Tereasaterebene JComponents can returns any real size after pack() as you know, the XXSizes are just hints to the LayoutManager, which has complete control over whether or not to respect them :-) The hints as such are the responsibility of the component itself which should do their best to calculate the hints as accurately as they can. Nothing academic that I can see .. – Sisto

@Sisto final size can be set three ways, all three painting Rectangle with expected Dimension, I think that with the same CPU/GPU consuption (LayoutManagers are designated for), then result/maybe we can/are talking about good practicies, just academic discusion how it could be, without measurable impact by running on modern PC with todays Native OS and latest Java6/7 – Tereasaterebene

@Tereasaterebene final size can be set three ways - back to our usual relationship: I don't have the faintest idea what you are talking about ;-) There is exactly one way to set the size of a component - either done by the LayoutManager or manually - and that's setBounds(...), so you must mean something else. – Sisto

@Tereasaterebene no war intended - removed the rubbish comment – Sisto

Answer 1

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.

Sisto

I don't understand your last sentence very well, my english is not the best, i have to read it twice everything, setPrefferedSize exists since 1.5 in java.awt.Component so it's a bound property since that, so in example you change your label and prefferedSize is changed but it's not changed by setPreferredSize, but as is from 1.5 how this could happen is not an old method – Confrere

@Confrere my point is that it is not a bound property because it doesn't comply to its contract as defined by beans spec, the doc is simply wrong. – Sisto

Perhaps the docs are right and the implementation is wrong :P – Confrere

Answer 2

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

Archon

+1 yes, very good described, can be guide for, but please read my last comment to kleopatra too – Tereasaterebene

+1 for the mighty warnings against setXXSize :-) Though I disagree with your advocation of consciously breaking method contracts - IMO that's just as evil as using inappropriately exposed api – Sisto

@Sisto I certainly don't disagree with you and I would hope its always possible to grow a object through inheritency that continues to honour the parent requirements, but I don't think that always possible, in the strictest sense. Some times (all be on rare occasions) it's nessaccery to alter a method in such away as to change the it. Good idea? That's contextual and in this particular context it would argue for it, as I don't see a particular use for the setter (or at least the setter being public). No, I'm not about to go breaking a bunch of method contracts on a whim - not today ;) – Archon

don't know wich answer accept both are equality good :) i just throw the coin – Confrere