Skip to content

Commit da40e24

Browse files
Refactor builder
There was some code duplication and unclear call delegation between methods of similar names.
1 parent dbc209a commit da40e24

4 files changed

Lines changed: 19 additions & 23 deletions

File tree

src/main/java/io/github/problem4j/core/AbstractProblemBuilder.java

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,14 @@
2020
*/
2121
package io.github.problem4j.core;
2222

23-
import io.github.problem4j.core.Problem.Extension;
2423
import java.io.Serializable;
2524
import java.net.URI;
2625
import java.util.ArrayList;
2726
import java.util.Collection;
2827
import java.util.HashMap;
2928
import java.util.List;
3029
import java.util.Map;
31-
import java.util.Objects;
3230
import java.util.Optional;
33-
import java.util.stream.Stream;
3431
import org.jspecify.annotations.Nullable;
3532

3633
/**
@@ -213,10 +210,12 @@ public ProblemBuilder extensions(@Nullable Map<String, @Nullable Object> extensi
213210
*/
214211
@Override
215212
public ProblemBuilder extensions(Problem.@Nullable Extension @Nullable ... extensions) {
216-
if (extensions != null && extensions.length > 0) {
217-
Stream.of(extensions)
218-
.filter(AbstractProblemBuilder::isExtensionValid)
219-
.forEach(e -> this.extensions.put(e.getKey(), Objects.requireNonNull(e.getValue())));
213+
if (extensions != null) {
214+
for (Problem.@Nullable Extension e : extensions) {
215+
if (e != null && e.getValue() != null) {
216+
this.extensions.put(e.getKey(), e.getValue());
217+
}
218+
}
220219
}
221220
return this;
222221
}
@@ -231,9 +230,7 @@ public ProblemBuilder extensions(Problem.@Nullable Extension @Nullable ... exten
231230
public ProblemBuilder extensions(
232231
@Nullable Collection<? extends Problem.@Nullable Extension> extensions) {
233232
if (extensions != null && !extensions.isEmpty()) {
234-
extensions.stream()
235-
.filter(AbstractProblemBuilder::isExtensionValid)
236-
.forEach(e -> this.extensions.put(e.getKey(), Objects.requireNonNull(e.getValue())));
233+
extensions(extensions.toArray(new Problem.Extension[0]));
237234
}
238235
return this;
239236
}
@@ -300,8 +297,4 @@ public String toString() {
300297

301298
return "ProblemBuilder{" + String.join(", ", entries) + "}";
302299
}
303-
304-
private static boolean isExtensionValid(@Nullable Extension extension) {
305-
return extension != null && extension.getValue() != null;
306-
}
307300
}

src/main/java/io/github/problem4j/core/Problem.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ static ProblemBuilder builder() {
7474
* @throws IllegalArgumentException if the {@code key} is {@code null}
7575
*/
7676
static Extension extension(@Nullable String key, @Nullable Object value) {
77+
// with JSpecify, the nullability check might not be necessary, kept to not break existing
78+
// behavior, may be revisited in future major versions
7779
if (key == null) {
7880
throw new IllegalArgumentException("key cannot be null");
7981
}

src/main/java/io/github/problem4j/core/ProblemBuilder.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ public interface ProblemBuilder {
123123
* @return this builder instance for chaining
124124
*/
125125
default ProblemBuilder extension(Problem.@Nullable Extension extension) {
126-
return extensions(extension);
126+
if (extension != null) {
127+
return extension(extension.getKey(), extension.getValue());
128+
}
129+
return this;
127130
}
128131

129132
/**

src/test/java/io/github/problem4j/core/ProblemImplTest.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ void givenSameProblems_shouldBeEqual() {
139139
Object problem;
140140
Object other;
141141

142-
problem = other = new ProblemImpl( "title", 404, "detail", mapOf("key", "value"));
142+
problem = other = new ProblemImpl("title", 404, "detail", mapOf("key", "value"));
143143

144144
assertThat(problem).isEqualTo(other);
145145
}
@@ -154,26 +154,24 @@ void givenTwoDifferentProblems_shouldNotBeEqual() {
154154

155155
@Test
156156
void givenProblemAndDifferentObject_shouldNotBeEqual() {
157-
Object problem = new ProblemImpl( "title1", 404, "detail1", mapOf("key", "value"));
157+
Object problem = new ProblemImpl("title1", 404, "detail1", mapOf("key", "value"));
158158
Object differentObject = "always wanted to be a problem";
159159

160160
assertThat(problem).isNotEqualTo(differentObject);
161161
}
162162

163163
@Test
164164
void givenTwoEqualProblems_shouldHaveSameHashCode() {
165-
Problem problem1 = new ProblemImpl( "title", 404, "detail", mapOf("key", "value"));
166-
Problem problem2 = new ProblemImpl( "title", 404, "detail", mapOf("key", "value"));
165+
Problem problem1 = new ProblemImpl("title", 404, "detail", mapOf("key", "value"));
166+
Problem problem2 = new ProblemImpl("title", 404, "detail", mapOf("key", "value"));
167167

168168
assertThat(problem1.hashCode()).isEqualTo(problem2.hashCode());
169169
}
170170

171171
@Test
172172
void givenTwoDifferentProblems_shouldHaveDifferentHashCodes() {
173-
Problem problem1 =
174-
new ProblemImpl( "title1", 404, "detail1", mapOf("key", "value1"));
175-
Problem problem2 =
176-
new ProblemImpl( "title2", 500, "detail2", mapOf("key", "value2"));
173+
Problem problem1 = new ProblemImpl("title1", 404, "detail1", mapOf("key", "value1"));
174+
Problem problem2 = new ProblemImpl("title2", 500, "detail2", mapOf("key", "value2"));
177175

178176
assertThat(problem1.hashCode()).isNotEqualTo(problem2.hashCode());
179177
}

0 commit comments

Comments
 (0)