Refactor: improve code quality by addressing code smells

problem-gson/src/main/java/org/zalando/problem/gson/StatusTypeAdapter.java

@@ -15,7 +15,11 @@
 @AllArgsConstructor
 final class StatusTypeAdapter extends TypeAdapter<StatusType> {
 
-    private Map<Integer, StatusType> index;
+    /**
+     * Lookup table mapping HTTP status codes to StatusType instances.
+     * Immutable after construction.
+     */
+    private final Map<Integer, StatusType> index;
 
     @Override
     public void write(
@@ -41,7 +45,10 @@ public StatusType read(final JsonReader in) throws IOException {
 
         final int statusCode = in.nextInt();
         @Nullable final StatusType status = index.get(statusCode);
-        return status == null ? new UnknownStatus(statusCode) : status;
+
+        return status == null
+                ? new UnknownStatus(statusCode)
+                : status;
     }
 
-}
+}

problem/src/main/java/org/zalando/problem/DefaultProblem.java

@@ -11,23 +11,29 @@
 @API(status = STABLE)
 public final class DefaultProblem extends AbstractThrowableProblem {
 
-    // TODO needed for jackson
+    /**
+     * Constructor used by Jackson during JSON deserialization.
+     * <p>
+     * This constructor matches the property-based binding required by Jackson.
+     * For programmatic creation of {@link Problem} instances, prefer using
+     * {@link Problem#builder()}.
+     */
     DefaultProblem(@Nullable final URI type,
-            @Nullable final String title,
-            @Nullable final StatusType status,
-            @Nullable final String detail,
-            @Nullable final URI instance,
-            @Nullable final ThrowableProblem cause) {
+                   @Nullable final String title,
+                   @Nullable final StatusType status,
+                   @Nullable final String detail,
+                   @Nullable final URI instance,
+                   @Nullable final ThrowableProblem cause) {
         super(type, title, status, detail, instance, cause);
     }
 
     DefaultProblem(@Nullable final URI type,
-            @Nullable final String title,
-            @Nullable final StatusType status,
-            @Nullable final String detail,
-            @Nullable final URI instance,
-            @Nullable final ThrowableProblem cause,
-            @Nullable final Map<String, Object> parameters) {
+                   @Nullable final String title,
+                   @Nullable final StatusType status,
+                   @Nullable final String detail,
+                   @Nullable final URI instance,
+                   @Nullable final ThrowableProblem cause,
+                   @Nullable final Map<String, Object> parameters) {
         super(type, title, status, detail, instance, cause, parameters);
     }
-}
+}

problem/src/main/java/org/zalando/problem/Problem.java

@@ -58,9 +58,9 @@ default StatusType getStatus() {
     }
 
     /**
-     * A human readable explanation specific to this occurrence of the problem.
+     * A human-readable explanation specific to this occurrence of the problem.
      *
-     * @return A human readable explaination of this problem
+     * @return a human-readable explanation of this problem
      */
     @Nullable
     default String getDetail() {
@@ -107,7 +107,10 @@ static ThrowableProblem valueOf(final StatusType status, @Nullable final URI ins
     static ThrowableProblem valueOf(final StatusType status,
                                     @Nullable final String detail,
                                     @Nullable final URI instance) {
-        return GenericProblems.create(status).withDetail(detail).withInstance(instance).build();
+        return GenericProblems.create(status)
+                .withDetail(detail)
+                .withInstance(instance)
+                .build();
     }
 
     /**
@@ -122,10 +125,10 @@ static ThrowableProblem valueOf(final StatusType status,
      *   // Returns "about:blank{404, Not Found, instance=https://example.org/}"
      *   Problem.valueOf(NOT_FOUND, URI.create("https://example.org/")).toString();
      *
-     *   // Returns "about:blank{404, Not Found, Order 123, instance=https://example.org/"}
+     *   // Returns "about:blank{404, Not Found, Order 123, instance=https://example.org/}"
      *   Problem.valueOf(NOT_FOUND, "Order 123", URI.create("https://example.org/")).toString();
      *
-     *   // Returns "https://example.org/problem{422, Oh, oh!, Crap., instance=https://example.org/problem/123}
+     *   // Returns "https://example.org/problem{422, Oh, oh!, Crap., instance=https://example.org/problem/123}"
      *   Problem.builder()
      *       .withType(URI.create("https://example.org/problem"))
      *       .withTitle("Oh, oh!")
@@ -144,18 +147,32 @@ static ThrowableProblem valueOf(final StatusType status,
      * @see Problem#valueOf(StatusType, String, URI)
      */
     static String toString(final Problem problem) {
-        final Stream<String> parts = Stream.concat(
-                Stream.of(
-                        problem.getStatus() == null ? null : String.valueOf(problem.getStatus().getStatusCode()),
-                        problem.getTitle(),
-                        problem.getDetail(),
-                        problem.getInstance() == null ? null : "instance=" + problem.getInstance()),
-                problem.getParameters()
-                        .entrySet().stream()
-                        .map(Map.Entry::toString))
-                .filter(Objects::nonNull);
-
-        return problem.getType().toString() + "{" + parts.collect(joining(", ")) + "}";
+
+        final String statusCode = problem.getStatus() == null
+                ? null
+                : String.valueOf(problem.getStatus().getStatusCode());
+
+        final String instancePart = problem.getInstance() == null
+                ? null
+                : "instance=" + problem.getInstance();
+
+        final Stream<String> standardParts = Stream.of(
+                statusCode,
+                problem.getTitle(),
+                problem.getDetail(),
+                instancePart
+        );
+
+        final Stream<String> customParts = problem.getParameters()
+                .entrySet()
+                .stream()
+                .map(Map.Entry::toString);
+
+        final String body = Stream.concat(standardParts, customParts)
+                .filter(Objects::nonNull)
+                .collect(joining(", "));
+
+        return problem.getType() + "{" + body + "}";
     }
 
-}
+}

problem/src/main/java/org/zalando/problem/ProblemBuilder.java

@@ -4,8 +4,8 @@
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.net.URI;
-import java.util.Arrays;
-import java.util.HashSet;
+import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
@@ -15,9 +15,23 @@
 @API(status = STABLE)
 public final class ProblemBuilder {
 
-    private static final Set<String> RESERVED_PROPERTIES = new HashSet<>(Arrays.asList(
-            "type", "title", "status", "detail", "instance", "cause"
-    ));
+    private static final String PROP_TYPE = "type";
+    private static final String PROP_TITLE = "title";
+    private static final String PROP_STATUS = "status";
+    private static final String PROP_DETAIL = "detail";
+    private static final String PROP_INSTANCE = "instance";
+    private static final String PROP_CAUSE = "cause";
+
+    private static final Set<String> RESERVED_PROPERTIES =
+            Collections.unmodifiableSet(
+                    new LinkedHashSet<>(java.util.Arrays.asList(
+                            PROP_TYPE,
+                            PROP_TITLE,
+                            PROP_STATUS,
+                            PROP_DETAIL,
+                            PROP_INSTANCE,
+                            PROP_CAUSE
+                    )));
 
     private URI type;
     private String title;

problem/src/main/java/org/zalando/problem/ThrowableProblem.java

@@ -13,8 +13,9 @@
 import static org.zalando.problem.spi.StackTraceProcessor.COMPOUND;
 
 /**
- * 
- * {@link Problem} instances are required to be immutable.
+ * Base class for exceptions implementing {@link Problem}.
+ *
+ * <p>{@link Problem} instances are required to be immutable.</p>
  */
 @API(status = STABLE)
 public abstract class ThrowableProblem extends RuntimeException implements Problem, Exceptional {
@@ -33,13 +34,22 @@ protected ThrowableProblem(@Nullable final ThrowableProblem cause) {
     @Override
     public String getMessage() {
         return Stream.of(getTitle(), getDetail())
-            .filter(Objects::nonNull)
-            .collect(joining(": "));
+                .filter(Objects::nonNull)
+                .collect(joining(": "));
     }
 
+    /**
+     * Returns the cause of this problem as a {@link ThrowableProblem}.
+     *
+     * <p>The cast is safe because the only constructor that accepts a cause
+     * requires it to already be a {@code ThrowableProblem}. Therefore,
+     * {@code super.getCause()} cannot return any other exception type.</p>
+     *
+     * @return the cause of this problem, or {@code null} if no cause exists
+     */
     @Override
+    @SuppressWarnings("unchecked")
     public ThrowableProblem getCause() {
-        // cast is safe, since the only way to set this is our constructor
         return (ThrowableProblem) super.getCause();
     }
 
@@ -48,4 +58,4 @@ public String toString() {
         return Problem.toString(this);
     }
 
-}
+}

problem/src/test/java/org/zalando/problem/JunitStackTraceProcessor.java

@@ -1,33 +1,33 @@
 package org.zalando.problem;
 
-import static java.util.stream.Collectors.toList;
-
 import java.util.Collection;
+
 import org.zalando.problem.spi.StackTraceProcessor;
 
+import static java.util.stream.Collectors.toList;
+
 public final class JunitStackTraceProcessor implements StackTraceProcessor {
 
     @Override
-    public Collection<StackTraceElement> process(
-        final Collection<StackTraceElement> elements
-    ) {
-        return elements
-            .stream()
-            .filter(element -> !isJunitStackTrace(element))
-            .collect(toList());
+    public Collection<StackTraceElement> process(final Collection<StackTraceElement> elements) {
+        return elements.stream()
+                .filter(element -> !isJunitStackTrace(element))
+                .collect(toList());
     }
 
+    /**
+     * Determines whether a stack trace element originates from JUnit.
+     *
+     * @param element the stack trace element to inspect
+     * @return {@code true} if the element belongs to a JUnit class or module;
+     *         {@code false} otherwise
+     */
     private boolean isJunitStackTrace(final StackTraceElement element) {
-        final String className = element.getClassName();
-        // Filter by class name - catch all JUnit packages
-        if (className.startsWith("org.junit.")) {
-            return true;
-        }
-        // Filter by module name (Java 9+)
         final String moduleName = element.getModuleName();
-        if (moduleName != null && moduleName.startsWith("org.junit.")) {
-            return true;
-        }
-        return false;
+
+        return element.getClassName().startsWith("org.junit.")
+                || (moduleName != null
+                && moduleName.startsWith("org.junit."));
     }
-}
+
+}