Fleshgrinder commented on a change in pull request #9370:
URL: https://github.com/apache/kafka/pull/9370#discussion_r499643570



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -4160,15 +4231,18 @@ void handleFailure(Throwable throwable) {
     }
 
     @Override
-    public DescribeUserScramCredentialsResult 
describeUserScramCredentials(List<String> users, 
DescribeUserScramCredentialsOptions options) {
+    public @NotNull DescribeUserScramCredentialsResult 
describeUserScramCredentials(
+        final @Nullable List<@NotNull String> users,
+        final @NotNull DescribeUserScramCredentialsOptions options
+    ) {
         final KafkaFutureImpl<DescribeUserScramCredentialsResponseData> 
dataFuture = new KafkaFutureImpl<>();
         final long now = time.milliseconds();
         Call call = new Call("describeUserScramCredentials", 
calcDeadlineMs(now, options.timeoutMs()),
                 new LeastLoadedNodeProvider()) {
             @Override
             public DescribeUserScramCredentialsRequest.Builder 
createRequest(int timeoutMs) {
                 return new DescribeUserScramCredentialsRequest.Builder(
-                        new 
DescribeUserScramCredentialsRequestData().setUsers(users.stream().map(user ->
+                        new 
DescribeUserScramCredentialsRequestData().setUsers(users == null ? 
Collections.emptyList() : users.stream().map(user ->

Review comment:
       This way we would also run the empty map through _stream_, which is not 
required. I tried to make the change with as little impact as possible but if I 
were to write it then I would do it as follows:
   
   ```java
           Call call = new Call("describeUserScramCredentials", 
calcDeadlineMs(now, options.timeoutMs()), new LeastLoadedNodeProvider()) {
               @Override
               public DescribeUserScramCredentialsRequest.Builder 
createRequest(final int timeoutMs) {
                   final DescribeUserScramCredentialsRequestData requestData = 
new DescribeUserScramCredentialsRequestData();
   
                   if (users != null) {
                       final List<UserName> userNames = new 
ArrayList<>(users.size());
                       for (final String user : users) {
                           userNames.add(new UserName().setName(user));
                       }
                       requestData.setUsers(userNames);
                   }
   
                   return new 
DescribeUserScramCredentialsRequest.Builder(requestData);
               }
   ```
   
   This is more code, yes, and it is not using _stream_ anymore but it is both 
clearer, faster, and allocates less.




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to