[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6268: NIFI-10244 Add nifi-web-client-api and implementation

[GitBox <gi...@apache.org>]

exceptionfactory commented on code in PR #6268:
URL: https://github.com/apache/nifi/pull/6268#discussion_r942441710


##########
nifi-commons/nifi-web-client/src/main/java/org/apache/nifi/web/client/StandardHttpEntityHeaders.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.client;
+
+import org.apache.nifi.web.client.api.HttpEntityHeaders;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Standard implementation of HTTP Entity Headers for Standard Web Client Service
+ */
+class StandardHttpEntityHeaders implements HttpEntityHeaders {
+    private final Map<String, List<String>> headers;
+
+    StandardHttpEntityHeaders(final Map<String, List<String>> headers) {
+        this.headers = headers;
+    }
+
+    @Override
+    public Optional<String> getFirstHeader(final String headerName) {
+        final List<String> values = getHeader(headerName);
+        return values.stream().findFirst();
+    }
+
+    @Override
+    public List<String> getHeader(final String headerName) {
+        Objects.requireNonNull(headerName, "Header Name required");
+        final List<String> values = headers.get(headerName);
+        return values == null ? Collections.emptyList() : Collections.unmodifiableList(values);

Review Comment:
   That's a good question. It depends on the particular use case, but in general for collections, it is better to return an empty instead of null to simplify response checking. There may be cases where null and empty mean different things, but in this case, there does not seem to be any value is returning a null versus and an empty list.
   
   As far as the functional style versus the ternary null check, there doesn't seem to be a consistent pattern. For this particular case, there doesn't seem to be much different in readability, so I could go either way. I will leave it for now, but will keep the suggestion in mind. There are certainly other places in the application where the functional style is used, and provides a concise implementation.



-- 
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-unsubscribe@nifi.apache.org

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