IndexedContainer inline classes

The inline classes IndexedContainerItem and IndexedContainerProperty inside com.itmill.toolkit.data.util.IndexedContainer seem to be private. Shouldn’t it be better to have them at least protected (if not public), because otherwise they are unreachable if you start doing your own container, extending the IndexedContainer?

So, either I’ve missed something, or I need to copy&paste the whole IndexedContainer.java to get something similar of my own, with my own tweaks.

The reasoning for keeping IndexedContainer internal implementation private was to allow it to change without breaking extending classes. As the container has been already implemented in year 2002, it could be said to be mature and thus we could change private to protected to make it extendable. Good idea.

Added http://dev.itmill.com/ticket/1732

Hi!

As Joonas said, it is often a good practice to hide everything you can and open it for extension if it has really been designed for it. Check out to code in GWT widgets. Google coders have really taken this far. It is awful to extend some of their widgets, but can’t really blame them that their widgets don’t work as they should.

How exactly have you planned to extend IndexedContainer ? Item and Property inner classes in IndexedContainer are basically proxies to values stored in IndexedContainer. What is your point to extend them?

cheers,
matti

We use the IndexedContainer as a starting point for our own needs. In case we need to instantiate e.g. an IndexedContainerItem, we can’t, because the constructor is private.

OTOH, we might not need to (at this stage of the modifications, at least), but you never know. It just seems funny that you can extend the IndexedContainer and use whatever it provides, but not the Items and Properties it provides.

I would try to avoid cut-n-pasting the whole thing. IndexedContainer is fairly large, but really well tested class. If you can avoid regressions by extending it would be great.

If the private inner classes become a blocker, you can then cut-n-paste the whole thing, only change the protection of those inner classes and contiue to extend this “ProtectedIndexedContainer”…

And please submit diff from IndexedContainer to ProtectedIndexedContainer to http://dev.itmill.com/ticket/1732