Review request for JDK-8156680: jdeps implementation refresh

Mandy Chung mandy.chung at oracle.com
Wed May 11 19:44:07 UTC 2016


> On May 11, 2016, at 10:39 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Mandy,
> 
> I had a look at the first webrev.
> 
> Code reorganization in some of the files makes it difficult
> to follow what is going on. I will try to import
> the changes in my local workspace tomorrow and play with
> it a bit.

Thanks.

> 
> I have two small comments so far:
> 
> Archive.java:
> 
> 106     public Stream<Location> getDependencies() {
> 107         return deps.values().stream()
> 108                    .flatMap(Set::stream);
> 109     }
> 
> I suspect it might be more correct to do:
> 
>         return deps.values().stream()
>                        .flatMap(Set::stream)
>                        .distinct();
> 

This lets the caller of this method to decide if it wants to sort the stream, distince, or filtering to avoid unnecessary sort.

> DependencyFinder.java:
> 
>  82     Set<Archive> archives() {
>  83         waitForTasksCompleted();
>  84         Set<Archive> set = new LinkedHashSet<>();
>  85         parsedClasses.values().stream()
>  86             .forEach(archive -> set.add(archive));
>  87         return set;
>  88     }
> 
> Should that be:
> 
>       Set<Archive> archives() {
>           waitForTasksCompleted();
>           return parsedClasses.values().stream()
>               .toCollection(LinkedHashSet::new);
>       }
> 

Will modify that.

Mandy




More information about the core-libs-dev mailing list