hg: jdk7/tl/langtools: 6907575: [classfile] add support for classfile dependency analysis

[Rémi Forax]

Le 14/12/2009 23:54, Jonathan Gibbons a écrit :

[...]

> Rémi,
>
> I've been through your comments. The comment above is the most serious 
> and needs to be fixed. I'll also fix some of the minor issues 
> regarding visibility of fields.
>
> As for wildcards in return types, I think there is a time when it is 
> appropriate to use them and this is one of those times.  In 
> particular, they allow the use of covariant return types in overriding 
> methods, which I anticipate happening here if someone were to create a 
> finder that returns a iterable of a subtype of Dependency.  Also, 
> since in this case we're talking about an Iterable, most clients just 
> use Iterable objects in for loops, and don't need to worry about any 
> wildcard in the element type.

Ok, I have considered that findDependencies() will allways gather 
dependencies from
different sources hence have to collect them in a new data structure or 
at least
returns a view of already existing data structures.

I agree with you that using a wildcard will allow to use covariant 
return type,
but I don't see how you can use it here.
Dependencies.findAllDependencies() signature should be changed to propagate
the covariant type and for that the Finder should be parameterized by 
the type
of the dependency.

> Ultimately, like most of your other comments, I guess it comes down to 
> a matter of programming and API design style.

Yes, but it's important to have the same code convention in the whole JDK,
It avoids to have to rely on an IDE to find where a field is declared.
(I was reviewing this code in a suburb train on an ipod clone and
I'm afraid that it affect my mood at that time :)

>
> -- Jon
>

Rémi