On 08/29/2013 02:36 PM, Ed Merks wrote:
Not to
rain on anyone's parade, but respectfully suggest the platform
team should reconsider the gain verses the client impact for
generifying the JFace APIs. Designing generic containers isn't
easy, and it's all too easy to make mistakes that will be
difficult to correct in the future
[...]
In the end, many of the things being changed are effectively SPI.
One implements the APIs for providers and passes them to a generic
container that does the right things with it. There's little to
be gained from adding generics, to justify the cost to the
ecosystem, and getting it right is much harder than it appears at
first glance...
Although I trust people saying that adding generics is difficult and
error-prone (<T> or <? extends T> ?), I believe that
adding generics in JFace has a lot of value, simply because of the
number of cast necessary when dealing with a
ContentProvider/LabelProvider couple. Dealing with current APIs
forces us to remind the actual types for the JFace viewer, and it is
a tricky exercise that is error prone: I've often seen/written some
ClassCastException that takes more time to notice and debug that if
I could see immediate feedback in IDE thanks to generic.
Overall, adding generics to JFace will really boost productivity of
JFace users. That's IMO a valid reason to introduce warnings on
existing code.
What is the current issue? Is it that existing code using JFace will
now show warnings? Is this really an issue, does it prevent code to
work? I don't think so. Those warnings just reveals that current
usages of JFace don't leverage this new smart mechanism to prevent
from ClassCastException. It's not a big deal to see them.
IMO, just asking to roll back everything is just like disabling a
FindBugs rule: it doesn't solve anything, it just hides a place for
improvement. In the end, it reduce code quality.
So, IMHO, the balance is more in favor of adding generics that in
many cases will provide a better productivity and quality.
And I think it is totally fine to start merging such change so early
in the release train to get maximal feedback.
I see
(in Gerrit; thanks Matthias) that you've committed changes for
TreeViewer and here I think the whole approach is completely
questionable. When is it the case that a tree view has uniformly
the same elements throughout? I think that's so rarely the case
that it's worse than useless to make such an assumption; I would
argue it's mostly just noise that will never solve real problems.
I've seen many TreeViewers using a super type other than Object,
especially in the modeling world, where almost everything is an
EObject (and often a specialized super-type shared across the whole
metamodel), or in some JDT or PDE code where there are some
TreeNodeAdapter (or whatever pattern which create a dedicated type
to put in TreeContentProvider). So it seems to make sense for
TreeViewers as well. And for other cases, it's not a big deal to
write "TreeContentProvider<? extends Object>".
|