Timo,
Comments below.
On 31/07/2014 9:45 AM, Timo Kinnunen wrote:
Hi,
Regarding “Side-note about java.util.Collection's <T> T[] toArray(T[]
a): That method looks type-safe, but it's not.” and “But the point is,
its guaranteed to produce T[], and it's able to do so in any
Collection implementation. As we all know from writing c.toArray(new
X[c.size()]) again and again, it would have been totally convenient
if toArray return E[], but it doesn't though it certainly could, if
each collection constructor required an X[] prototype or an X.class
instance. ”
It really isn’t guaranteed to produce a T[]. An example:
List<Serializable> list = Arrays.asList("1", "2");
Serializable[] array = list.<Serializable> toArray(new
String[100][]); // throws ASE
array[0] = ""; // also throws ArrayStoreException
Here T is Serializable all the way (I explicitly say so!) but thanks
to String[] also being Serializable an assignment from String[][] to
Serializable[] is allowed. However the element type of the returned
array is still String[] at runtime and not the T. Generics and arrays
just don’t match.
Yes, they fundamentally don't match. And of course arrays have always
had these "odd" kinds of problems. E.g.,
Object[] array = new String[10];
array[0] = new Object();
It's never obvious that the array you're operating on might be far more
restrictive with respect to assignment than is obvious from its static type.
Regarding “So we'd need to all switch to Java 8 and our code would
become a sea of deprecation and raw type warnings.”
That’s what happens with Objects and raw types being passed around.
Switching to Java 8 seems orthogonal to using generics (a feature of
Java 1.5)...
You could also change existing methods to use strong types which would
break APIs but on the plus side get rid of @Deprecated and
@SuppressWarnings annotations littering the code.
Yes, I like to keep my code warning free. And prefer to remove uses of
deprecated things, though that runs counter to keeping EMF compatible
with Eclipse 3.5...
Regarding “So do the above look simple than the current code? Was it
easier to write? Did generics provide value or make it more complex
without providing value? It's clear It's not simple and generics
actually made it more complicated.
No only that, but as I've pointed out, the convenience to the author
is generics on the argument side of the equation, not on the return
type side.”
Well, this is how a strongly-typed IStructuredContentProvider might
look like in part:
interface IStructuredContentProvider<I, E> {
List<E> getElements(I inputElement);
}
Yes, that would have been a better design for JFace, though I would
suggest this as more convenient the following.
interface IStructuredContentProvider<I, E> {
Collection<? extends E> getElements(I inputElement);
}
I.e., callers should not expect to be able to add elements to the
returned list. And allowing the broader return type of Collection
wouldn't force client to convert collections to lists...
And a similarly strongly-typed ArrayContentProvider closely mimicking
the original (here using a Pair-helper class):
public final class ArrayContentProvider<E>
implements IStructuredContentProvider<Pair<E[],
Collection<E>>, E> {
@Override
public List<E> getElements(Pair<E[], Collection<E>>
inputElement) {
return
inputElement.lhs != null ?
Collections.unmodifiableList(Arrays.asList(inputElement.lhs))
: inputElement.rhs != null ?
Collections.unmodifiableList(new
ArrayList<>(inputElement.rhs))
: Collections.emptyList();
}
}
Of course that's not easily applied to the current class...
Using a Pair or a Tuple class in this manner is ugly. An improvement
would be to split that into two classes, an ArrayContentProvider and a
CollectionContentProvider:
class ArrayContentProvider<E>
implements IStructuredContentProvider<E[], E> {
public @Override List<E> getElements(E[] inputElement) {
return inputElement != null
? Collections.unmodifiableList(Arrays.asList(inputElement))
: Collections.emptyList();
}
}
class CollectionContentProvider<C extends Collection<E>, E>
implements IStructuredContentProvider<C, E> {
public @Override List<E> getElements(C inputElement) {
return inputElement != null
? Collections.unmodifiableList(new ArrayList<>(inputElement))
: Collections.emptyList();
}
}
Yes, much better though also not easily applied to the current class in
an erasure compatible way. And in the second case, wouldn't a design
that allows inputElement to be returned be much nicer (and more
efficient) instead of the copying and wrapping you've shown here. If
the intent is for the returned value to be unmodifiable, making that
clear in the type signature would be better too...
If everything was this strongly-typed then these classes are actually
almost useless. The callers could just as well call
Collections.unmodifiableList(Arrays.asList(inputElement)) and
Collections.unmodifiableList(new ArrayList<>(inputElement)) themselves.
This would be the best-case scenario and is unlikely to happen, but
with generics it could. So if you don’t see a clear benefit from
generics, add more generics!
If the current implementation were not heavily array focused, it would
have been amenable to generification, but that's not the case.
--
Have a nice day,
Timo.
Sent from Windows Mail
*From:* Ed Merks <mailto:[email protected]>
*Sent:* Thursday, July 31, 2014 08:35
*To:* [email protected]
<mailto:[email protected]>
Timo,
Comments below.
On 30/07/2014 7:19 PM, Timo Kinnunen wrote:
Hi,
Regarding the ArrayContentProvider, something like this seems to
compile just fine:
interface IStructuredContentProvider<T> {
default @Deprecated Object[] getElements(Object inputElement) {
return elements(inputElement).toArray();
}
So we'd need to all switch to Java 8 and our code would become a sea
of deprecation and raw type warnings.
ArrayList<T> elements(Object inputElement);
Of course you'd propose Collection<? extends T> or List<? extends T>
not ArrayList, right?
}
class ArrayContentProvider<T> implements
IStructuredContentProvider<T> {
private static ArrayContentProvider<?> instance;
public static <T> ArrayContentProvider<T> getInstance() {
synchronized (ArrayContentProvider.class) {
if (instance == null) {
instance = new ArrayContentProvider<>();
}
@SuppressWarnings("unchecked")
ArrayContentProvider<T> result =
(ArrayContentProvider<T>) instance;
return result;
}
}
public @Deprecated @Override Object[] getElements(Object
inputElement) {
return elements(inputElement).toArray();
}
So you're suggesting that no one should ever use or implement this
method. While that's certainly a way to avoid the whole problem with
generic array, none of this fits with Dani's "the problems should go
way just by inferring types".
public @Override ArrayList<T> elements(Object inputElement) {
if (inputElement instanceof Object[]) {
@SuppressWarnings("unchecked")
List<T> list = Arrays.asList((T[]) inputElement);
return new ArrayList<>(list);
}
if (inputElement instanceof Collection) {
@SuppressWarnings("unchecked")
Collection<T> collection = (Collection<T>) inputElement;
return new ArrayList<>(collection);
}
return new ArrayList<>();
}
}
So do the above look simple than the current code? Was it easier to
write? Did generics provide value or make it more complex without
providing value? It's clear It's not simple and generics actually
made it more complicated.
No only that, but as I've pointed out, the convenience to the author
is generics on the argument side of the equation, not on the return
type side.
public class Snippet {
public static void main(String[] args) {
ArrayContentProvider<String> instance =
ArrayContentProvider.getInstance();
ArrayList<String> elements1 = instance.elements(null);
System.out.println(elements1);
}
}
Yeah, there’s a lot of unchecked casts but at least they are
visible and localized. That’s just the price one has to pay for
weak typing.
The real question is, was it easier to write? Because in the end, who
are the callers to these APIs? A bunch of generic viewers that are
already written and that we don't generally write ourselves. And they
don't care currently. So the whole things fundamentally has to come
down to what's easiest for the authors to write content providers.
Otherwise I agree with trying to make Object[] generic not working
at all. Arrays in Java are just too strongly and wrongly typed to
work well with generics. Best just to deprecate them and replace
them with proper collections that are much nicer to work with.
Yes, it's certainly far more reasonable to argue getting rid of the
arrays. But that's even more fundamentally disruptive, and requires
Java 8. Note that EMF heavily uses Collection<?> rather than Object[]
in its platform neutral equivalents of these APIs.
That’s also why I picked ArrayList<> rather than some other List<>
as the return type, to make it obvious to the caller that they are
free to do anything and everything with the returned collection.
I think that makes no sense. The client must always create a specific
list implementation and the contract allows the caller to add
instances to that collection? Collection<? extends T> would seem far
more reasonable and convenient. Then one could return just the
collection you already have or use Arrays.asList.
--
Have a nice day,
Timo.
Sent from Windows Mail
*From:* Ed Merks <mailto:[email protected]>
*Sent:* Wednesday, July 30, 2014 17:46
*To:* [email protected]
<mailto:[email protected]>
Dani,
As I've made pretty clear in the past, I'm a huge non-fan of this
effort. I find it ironic that the platform is rife with raw
types (List), and rather than investing an effort to eliminate
those, effort will be invested on generifying things that just
aren't screaming to be generified, and to do so in a context that
is heavily dominated by Object[], which interacts exceedingly
poorly with generics. Thought I've made that argument already, I
think it bears repeating...
Anywhere you see something like T[] where T is a type parameter,
you already have to get suspicious. Of course we see this in
java.util.Collection:
<T> T[] toArray(T[] a);
But note that T is derived from the argument, and argument my not
be null, so even in an erased runtime environment we can determine
a reasonable T from "a", so suspicion alleviated and that's why we
can have generic collection implementations...
Note that however nice it would have been that toArray() method
looked like this:
E[] toArray()
rather than
Object[] toArray
because you can't implement this generically, unless you provide
some object with the necessary *runtime *information about E to
the constructor of an implementation class...
So consider this:
public interface IStructuredContentProvider extends IContentProvider {
public Object[] getElements(Object inputElement);
}
there's no relationship between the input element's type and the
type of the array, so if someone proposes
public interface IStructuredContentProvider<X, Y> extends
IContentProvider<X> {
public Y[] getElements(X inputElement);
}
I'm going to be very suspicious. What this tells me is there must
be some sensible way of being sure that I'll be getting back a
real Y[] instance and not an Object[] instance. What could that
sensible way be?
Of course directly implementing that interface in a sensible way
is a good solution, but what about generic solutions?
Consider org.eclipse.jface.viewers.ArrayContentProvider (and note
that EMF generally has *only *content provider implementations
analogous to this). This existing implementation just don't
work. You can just forget about
org.eclipse.jface.viewers.ArrayContentProvider.instance, you can
just deprecate
org.eclipse.jface.viewers.ArrayContentProvider.getInstance(), and
you'd better add a public constructor and deprecate it while your
at it, because this implementation
public Object[] getElements(Object inputElement) {
if (inputElement instanceof Object[]) {
return (Object[]) inputElement;
}
if (inputElement instanceof Collection) {
return ((Collection) inputElement).toArray();
}
return new Object[0];
}
simply doesn't work for collections. You'll need new
constructors and new getInstance methods each of which specify the
array type, either as java.lang.Class or as an array prototype, as
in the first form to toArray above. You'd have to provide that
even for the constructor that takes a collection, just the
collection instance will not suffice.
Great, so much for adding generics to JFace being erasure
compatible, counter to what was the case when Java's collection
library was generified. Nothing in the collections library was
deprecated and no new methods were added. In other words, for
JFace's changes, you can't just turn off warnings about raw types,
you must deal with the deprecations and API changes. So if you're
trying to maintain something that works with old versions of
Eclipse (i.e., EMF is compatible with Eclipse 3.5), you're
completely hosed. You can't add generics, because you can't
compile against an older target platform that does have it, so you
simply have to live with a sea of raw type warnings (or turn them
all off and lose the value of even having such warnings). Also,
you can't start using the new methods instead of the deprecated
ones, so you have to live with that sea of warning as well, or
just turn them off too. Nor you can you exploit any of it to
provide value to clients (given the premise there is value),
because you can't reasonably address this ArrayContentProvider
problem without inflicting the same pain on the clients (who
actually have better things to do, go figure).
Even the premise that this effort has value is questionable.
Granted, someone writing their first content provider might find
it useful, if (and only if) it's one that's concrete and if (and
only if) it doesn't need to deal with several input types that
have no common super type (and even in that case they'll still
typically end up with instanceof tests to return
subtype-appropriate results). That's on the argument side of the
getElements. On the return type side, it's of no benefit to the
author; they just pass this content provider into a generic viewer
that doesn't care whether it's Object[] or X[]. So in fact it's
just a burden with which one must conform.
So is the value of this whole exercise eliminating instance of
checks for the arguments of the provider implementations? Does
this tangible (but small) benefit justify the impact on the long
established community? Do we expect that community to eliminate
their deprecations and generify all their code? (Sorry guys and
girls, all your existing toArray() calls are invalid, or, don't
worry about it, just sprinkle <Object, Object> everywhere.) I
wonder, will JDT and PDE do that? If not, why expect the rest of
the community to do it? And if you don't expect that, what
exactly are you expecting will come from this?
I suggest folks carefully weight the benefits against the
disruptive nature of this type of change.
Regards,
Ed
On 30/07/2014 11:42 AM, Daniel Megert wrote:
Just for the records, here are some constraints that I
required in order to agree to continue that work:
- Some stuff just doesn't make sense to be generified because
it often contains various kinds of objects, e.g. (tree)
viewers. See also
_http://dev.eclipse.org/mhonarc/lists/platform-ui-dev/msg05459.html_.
- If generified types cannot be plugged together unless
everything is again just Object or Class, it's not worth to
generify those types.
- The generified code must be in a shape so that clients can
start to fix their code by invoking Refactor > Infer Generic
Type Arguments... This needs to be validate on existing
Platform UI code.
Dani
From: Lars Vogel <[email protected]>
To: [email protected], Jeanderson Candido
<[email protected]>, Hendrik Still <[email protected]>
Date: 30.07.2014 11:23
Subject: [cross-project-issues-dev] Information about the
"Generifing JFace viewers" project
Sent by: [email protected]
------------------------------------------------------------------------
Hi,
as some of you probably remember, the platform.ui team started
a GSoC project last year to generify the JFace viewer
framework. We (platform.ui team together with John Arthone and
Dani Megert) decided that it is worth to finish this project
and started a new GSoC project.
Jeanderson Barros Candido (cc) is working on this project with
Hendrik Still (cc) (GSoC student from last year) and me as
mentor.
I personally think the work looks already very good and plan
to integrated it soon into the master. We are trying to learn
from the experience from last year, therefore:
- We plan to integrate it as a whole, not piece wise so
people can fix warning messages created by this change
- We reworking the JFace snippets and tests at the same time
to have a first proof-point
- We plan to use it for platform views to validate that it works
Of course generifying an existing API, will result in certain
limitations and some suggested a complete rewrite of the JFace
viewer framework but this is currently not the scope of this
project.
The implementation is currently done at Github:
_https://github.com/jeandersonbc/eclipse.platform.ui_and we do
our planning in
_https://github.com/jeandersonbc/gsoc14-eclipse-planning_.
If someone wants to test the new implementation and provide
feedback, please let us know.
Best regards, Lars_______________________________________________
cross-project-issues-dev mailing list
[email protected]
To change your delivery options, retrieve your password, or
unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
cross-project-issues-dev mailing list
[email protected]
To change your delivery options, retrieve your password, or unsubscribe
from this list, visit
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
cross-project-issues-dev mailing list
[email protected]
To change your delivery options, retrieve your password, or unsubscribe
from this list, visit
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
cross-project-issues-dev mailing list
[email protected]
To change your delivery options, retrieve your password, or unsubscribe from
this list, visit
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
cross-project-issues-dev mailing list
[email protected]
To change your delivery options, retrieve your password, or unsubscribe from
this list, visit
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
