vdiravka commented on a change in pull request #1397: DRILL-6633: Replace usage 
of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r211213987
 
 

 ##########
 File path: common/src/test/java/org/apache/drill/test/SubDirTestWatcher.java
 ##########
 @@ -83,10 +84,10 @@
   private List<Path> subDirs;
 
   protected SubDirTestWatcher(File baseDir, boolean createAtBeginning, boolean 
deleteAtEnd, List<Path> subDirs) {
-    this.baseDir = Preconditions.checkNotNull(baseDir);
+    this.baseDir = Objects.requireNonNull(baseDir);
     this.createAtBeginning = createAtBeginning;
     this.deleteAtEnd = deleteAtEnd;
-    this.subDirs = Preconditions.checkNotNull(subDirs);
+    this.subDirs = Objects.requireNonNull(subDirs);
 
     Preconditions.checkArgument(!subDirs.isEmpty(), "The list of subDirs is 
empty.");
 
 Review comment:
   `Objects.requireNonNull` is introduced mostly for `Streams`, but not for 
replacing Guava Preconditions.
   From other hand they have the same implementation. Except 
`Preconditions.checkNotNull()` overloaded method with NPE error message 
formatting and Guava recommends to use Preconditions, see [Guava 
master](https://github.com/google/guava/blob/master/guava/src/com/google/common/base/Preconditions.java#L93).
   
   Looks like Calcite project replaces these methods, see [Calcite 
master](https://github.com/apache/calcite/blob/3fa29455664bec0056c436491b369e0cd72242ea/src/main/config/forbidden-apis/signatures.txt#L58),
 but Apex doesn't replace them.
   
   Since we stay with Guava it could be reasonably to follow their 
documentation.
   But it is not critical, so I will leave the decision for you.
   
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to