You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/04/28 19:37:30 UTC
[GitHub] [accumulo] jmark99 opened a new pull request #1602: Lambdas replaced with method references.
jmark99 opened a new pull request #1602:
URL: https://github.com/apache/accumulo/pull/1602
This ticket updates instances of lambdas which can be replaced with method references. There was also one instance of an anonymous type being replaced with a method reference.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] jmark99 commented on a change in pull request #1602: Lambdas replaced with method references.
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #1602:
URL: https://github.com/apache/accumulo/pull/1602#discussion_r417277837
##########
File path: server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
##########
@@ -353,7 +353,7 @@ public Path matchingFileSystem(Path source, Set<String> options) {
URI optUri = URI.create(opt);
return sourceUri.getScheme().equals(optUri.getScheme())
&& Objects.equals(sourceUri.getAuthority(), optUri.getAuthority());
- }).map((String opt) -> new Path(opt)).findFirst().orElse(null);
+ }).map(Path::new).findFirst().orElse(null);
Review comment:
@ctubbsii I reverted this particular change based upon your suggestion. Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1602: Lambdas replaced with method references.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1602:
URL: https://github.com/apache/accumulo/pull/1602#discussion_r417005599
##########
File path: server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
##########
@@ -353,7 +353,7 @@ public Path matchingFileSystem(Path source, Set<String> options) {
URI optUri = URI.create(opt);
return sourceUri.getScheme().equals(optUri.getScheme())
&& Objects.equals(sourceUri.getAuthority(), optUri.getAuthority());
- }).map((String opt) -> new Path(opt)).findFirst().orElse(null);
+ }).map(Path::new).findFirst().orElse(null);
Review comment:
Path has strange and subtle differences between different constructors. The original code specified the constructor was the one that took a String explicitly. This code would work if the type was changed to URI, instead of String, but I'm not sure the behavior would be identical. The original version protects against subtle differences like that. For this reason, I'm not a big fan of method references for constructors, unless the type is *very* stable and well-known (like `ArrayList::new`).
The change you've made here is probably fine... but I'm not sure if we want to protect against possible subtle changes in this code by keeping the explicit String type declaration.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org