DefaultDockable change

Hi,

I think that the following change is the right thing for the DefaultDockable.
Currently getComponent() and getContentPane() both return the same object.
I think that getComponent() should return the component that was given as the parameter
in the constructor public DefaultDockable( Component component, String title, Icon icon )
If the framework doesn’t allow returning value of getComponent() to be null, then there could be a check
that will return pane instead in this case.

I violently disagree :smiley:


I divide the classes and interfaces of the framework in different groups:

  • public API, that are the things clients directly interact with (do not change unless there is no other option)
  • protected API, maybe a clients sees these things, maybe not (change if necessary, try not to if possible)
  • private API, almost no client ever uses these things (can be changed at any time)

„Dockable“ and „DefaultDockable“ are part of the public API, and I preferrer to change the public API only if it is really necessary or if there is a huge benefit. I do not see either of these two things in your suggestion yet.


There are two ways how your suggestion can be understood:

  1. You just want another method to access your „component“ object. Incidentally this methods would have the same names (the whole issue is just a collision of names). But this component is put onto the content-pane, which can have many children. Your „getComponent“ method would just be a shortcut for „getContentPane().getComponent(0)“, and that method just doesn’t add any value to the framework at all (it would be a special method useful for only one situation).

  2. You want to replace the old „getComponent“ with a new „getComponent“.

  • Well… the root-Component of a Dockable has some constraints: it has to be a focus cycle root, and it should support the „BackgroundPaint“ mechanism. It is possible that in the future this component has to support even more special, framework specific behavior. I would not burden the client with these tasks.
  • The most important point is backwards compatibility: in your suggestion the content-pane would be obsolete - but there are already clients using the content-pane. All of them would need to be updated.

Some minor issues are:

  • The root-component must never be replaced, you can add and remove components from the content-pane whenever you want.
  • getContentPane and getComponent currently return the same object, but it could be that in the future the content-pane is just a child of the component. So „getContentPane“ can also be seen as a precausion for backwards compatibility.

Finally:

  • The weight of the content-pane is small, a JPanel costs only a few bytes. As an optimization this would not help much.
  • If someone is really desperate to get rid of the content-pane: subclassing AbstractDockable would give this developer more control.

Feel free to violently disagree too :stuck_out_tongue:

Regards
Beni

I understand your reasons from which the desire to not broke the public API is the most important.
My request was driven by the fact, that intuitively I thought that it is already working that way and was very surprised when got the ClassCastException :slight_smile:
My code (after a fix):

public EventsDockable() {
        super(new JScrollPane());
        eventsTable = new EventsTable();
        ((JScrollPane)this.getContentPane().getComponent(0)).setViewportView(eventsTable);
}```
I'm pretty sure that any developer looking at the API would think exactly as me - that getComponent() will return the component that was a parameter in the constructor. But given the fact, that I have not enough power to disagree violently :) maybe you can add getUserComponent() method instead? I hope you'll agree that code above will look better and be more intuitive in that case.

Well, I’ll add the method. It will be called “getFirstComponent” (because it is the first child that was added to the Dockable).

I’m not sure that every developer will agree with you, unfortunately there will not be enough people reading and answering to this thread…

Thanks.
But here are some thoughts that you might want to take into account.
Hope that we agree that I should have a getter for something that I give you as a parameter in the constructor. The way that it works now actually exposes me to the implementation details that you might want to change in the future. For this reason I think, that name that you suggested is not good. What if in the future the Control that I pass in the constructor will become a second component of the root pane? I think, that this method name should reflect purpose and not the implementation details.

Well, I can guarantee you that the all children of the content-pane will always be set by the client. That is set in stone because of backwards compatibility and because clients already have access to the content-pane (putting something onto it would be suicide as any client may just remove it…). The only thing that could happen, is that the root-component (of the Dockable) and the content-pane are no longer the same object. But in this case the content-pane still would not have any new children.

Actually the API of DefaultDockable is very much like the one of JFrame. And there this system with content-pane and root-pane worked quite well for several years.

OK, but the important thing is not the fact whether this change will occure or not. My point is like that. For me the DefaultDockable is like vignette. I believe that in 99% of cases it will hold single component. I see great usage of the method that returns me back the control that I wrapped up with the DefaultDockable. I don’t care about how exactly the DefaultDockable holds it. Your suggested method name actually exposes me to the inner structure of the DefaultDockable, which in this case not interesting to me. I just want my component back.

You are right with the name. I don’t like “getUserComponent” much because of the “user”, a “getClientComponent” would be acceptable.

Sounds very good. Lets make the deal. :wink: