elharo commented on a change in pull request #52:
URL: 
https://github.com/apache/maven-dependency-analyzer/pull/52#discussion_r811895120



##########
File path: 
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java
##########
@@ -201,48 +206,26 @@ else if ( file != null && file.isDirectory() )
         return artifactClassMap;
     }
 
-    private Set<String> buildTestDependencyClasses( MavenProject project ) 
throws IOException
+    private Set<String> buildTestOnlyDependencyClasses( Set<String> 
mainDependencyClasses,
+                                                        Set<String> 
testDependencyClasses )
     {
-        Set<String> testOnlyDependencyClasses = new HashSet<>();
-
-        String outputDirectory = project.getBuild().getOutputDirectory();
-        Set<String> nonTestDependencyClasses = new HashSet<>( 
buildDependencyClasses( outputDirectory ) );
-
-        String testOutputDirectory = 
project.getBuild().getTestOutputDirectory();
-        Set<String> testDependencyClasses = new HashSet<>( 
buildDependencyClasses( testOutputDirectory ) );
-
-        for ( String testString : testDependencyClasses )
-        {
-            if ( !nonTestDependencyClasses.contains( testString ) )
-            {
-                testOnlyDependencyClasses.add( testString );
-            }
-        }
-
-        return testOnlyDependencyClasses;
+        return testDependencyClasses.stream()

Review comment:
       By any chance do we already depend on Apache Commons Colelctions? If so, 
consider using SetUtils.difference which would read clearer. If not, add a 
comment like
   
   // Find classes used by tests not also used by the model code
   
   Or you could just use Set.removeAll to pull the mainDependencyClasses out 
and leave the test onlly classes behind. That's probably simplest.  
   

##########
File path: 
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java
##########
@@ -201,48 +206,26 @@ else if ( file != null && file.isDirectory() )
         return artifactClassMap;
     }
 
-    private Set<String> buildTestDependencyClasses( MavenProject project ) 
throws IOException
+    private Set<String> buildTestOnlyDependencyClasses( Set<String> 
mainDependencyClasses,

Review comment:
       I think this method can be static now

##########
File path: 
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java
##########
@@ -201,48 +206,26 @@ else if ( file != null && file.isDirectory() )
         return artifactClassMap;
     }
 
-    private Set<String> buildTestDependencyClasses( MavenProject project ) 
throws IOException
+    private Set<String> buildTestOnlyDependencyClasses( Set<String> 
mainDependencyClasses,
+                                                        Set<String> 
testDependencyClasses )
     {
-        Set<String> testOnlyDependencyClasses = new HashSet<>();
-
-        String outputDirectory = project.getBuild().getOutputDirectory();
-        Set<String> nonTestDependencyClasses = new HashSet<>( 
buildDependencyClasses( outputDirectory ) );
-
-        String testOutputDirectory = 
project.getBuild().getTestOutputDirectory();
-        Set<String> testDependencyClasses = new HashSet<>( 
buildDependencyClasses( testOutputDirectory ) );
-
-        for ( String testString : testDependencyClasses )
-        {
-            if ( !nonTestDependencyClasses.contains( testString ) )
-            {
-                testOnlyDependencyClasses.add( testString );
-            }
-        }
-
-        return testOnlyDependencyClasses;
+        return testDependencyClasses.stream()
+            .filter( klass -> !mainDependencyClasses.contains( klass ) )

Review comment:
       clazz is a little more common in my experience, as a fill-in for class. 
Though here it's just a string so perhaps className instead? 

##########
File path: 
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java
##########
@@ -201,48 +206,26 @@ else if ( file != null && file.isDirectory() )
         return artifactClassMap;
     }
 
-    private Set<String> buildTestDependencyClasses( MavenProject project ) 
throws IOException
+    private Set<String> buildTestOnlyDependencyClasses( Set<String> 
mainDependencyClasses,

Review comment:
       perhaps change "build" to "list" or "find" since you're not creating the 
objects here, which builder methods usually do




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to