Skip to content

Commit 1f701d0

Browse files
jeffgbutlerjeffgbutler
committed
Small refactoring - don't return nulls!
1 parent 3377f93 commit 1f701d0

File tree

5 files changed

+37
-43
lines changed

5 files changed

+37
-43
lines changed

‎src/main/java/org/mybatis/dynamic/sql/util/FragmentAndParameters.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.HashMap;
1919
import java.util.Map;
2020
import java.util.Objects;
21+
import java.util.Optional;
2122

2223
public class FragmentAndParameters {
2324

@@ -69,5 +70,9 @@ public Builder withParameters(Map<String, Object> parameters) {
6970
public FragmentAndParameters build() {
7071
return new FragmentAndParameters(this);
7172
}
73+
74+
public Optional<FragmentAndParameters> buildOptional() {
75+
return Optional.of(build());
76+
}
7277
}
7378
}

‎src/main/java/org/mybatis/dynamic/sql/where/render/CriterionRenderer.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ private CriterionRenderer(Builder<T> builder) {
4242
}
4343

4444
public Optional<RenderedCriterion> render() {
45-
FragmentAndParameters initialCondition = renderCondition();
45+
Optional<FragmentAndParameters> initialCondition = renderCondition();
46+
4647
List<RenderedCriterion> subCriteria = sqlCriterion.mapSubCriteria(this::renderSubCriterion)
4748
.filter(Optional::isPresent)
4849
.map(Optional::get)
@@ -65,12 +66,11 @@ private <S> Optional<RenderedCriterion> renderSubCriterion(SqlCriterion<S> subCr
6566
.render();
6667
}
6768

68-
// caution - may return null
69-
private FragmentAndParameters renderCondition() {
69+
private Optional<FragmentAndParameters> renderCondition() {
7070
if (!sqlCriterion.condition().shouldRender()) {
71-
return null;
71+
return Optional.empty();
7272
}
73-
73+
7474
WhereConditionVisitor<T> visitor = WhereConditionVisitor.withColumn(sqlCriterion.column())
7575
.withRenderingStrategy(renderingStrategy)
7676
.withSequence(sequence)

‎src/main/java/org/mybatis/dynamic/sql/where/render/RenderedCriterion.java

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@
2626

2727
public class RenderedCriterion {
2828
private Optional<String> connector;
29-
private FragmentAndParameters initialCondition; // may be null
29+
private Optional<FragmentAndParameters> initialCondition;
3030
private List<RenderedCriterion> subCriteria;
3131

3232
private RenderedCriterion(Builder builder) {
3333
connector = Objects.requireNonNull(builder.connector);
34-
initialCondition = builder.initialCondition;
34+
initialCondition = Objects.requireNonNull(builder.initialCondition);
3535
subCriteria = Objects.requireNonNull(builder.subCriteria);
3636
}
3737

@@ -52,11 +52,8 @@ public FragmentAndParameters renderWithoutInitialConnector() {
5252
}
5353

5454
private FragmentCollector internalRender() {
55-
if (initialCondition == null) {
56-
return renderSubCriteriaOnly();
57-
} else {
58-
return renderConditionAndSubCriteria();
59-
}
55+
return initialCondition.map(this::renderConditionAndSubCriteria)
56+
.orElseGet(this::renderSubCriteriaOnly);
6057
}
6158

6259
private FragmentCollector renderSubCriteriaOnly() {
@@ -68,7 +65,7 @@ private FragmentCollector renderSubCriteriaOnly() {
6865
.collect(FragmentCollector.collect(initial));
6966
}
7067

71-
private FragmentCollector renderConditionAndSubCriteria() {
68+
private FragmentCollector renderConditionAndSubCriteria(FragmentAndParameters initialCondition) {
7269
return subCriteria.stream()
7370
.map(RenderedCriterion::renderWithInitialConnector)
7471
.collect(FragmentCollector.collect(initialCondition));
@@ -85,15 +82,15 @@ private String calculateFragment(FragmentCollector collector) {
8582

8683
public static class Builder {
8784
private Optional<String> connector;
88-
private FragmentAndParameters initialCondition;
85+
private Optional<FragmentAndParameters> initialCondition = Optional.empty();
8986
private List<RenderedCriterion> subCriteria = new ArrayList<>();
9087

9188
public Builder withConnector(Optional<String> connector) {
9289
this.connector = connector;
9390
return this;
9491
}
9592

96-
public Builder withInitialCondition(FragmentAndParameters initialCondition) {
93+
public Builder withInitialCondition(Optional<FragmentAndParameters> initialCondition) {
9794
this.initialCondition = initialCondition;
9895
return this;
9996
}
@@ -104,9 +101,10 @@ public Builder withSubCriteria(List<RenderedCriterion> subCriteria) {
104101
}
105102

106103
public Optional<RenderedCriterion> build() {
107-
if (initialCondition == null && subCriteria.isEmpty()) {
104+
if (!initialCondition.isPresent() && subCriteria.isEmpty()) {
108105
return Optional.empty();
109106
}
107+
110108
return Optional.of(new RenderedCriterion(this));
111109
}
112110
}

‎src/main/java/org/mybatis/dynamic/sql/where/render/WhereConditionVisitor.java

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
import org.mybatis.dynamic.sql.util.FragmentAndParameters;
3535
import org.mybatis.dynamic.sql.util.FragmentCollector;
3636

37-
public class WhereConditionVisitor<T> implements ConditionVisitor<T, FragmentAndParameters> {
37+
public class WhereConditionVisitor<T> implements ConditionVisitor<T, Optional<FragmentAndParameters>> {
3838

3939
private RenderingStrategy renderingStrategy;
4040
private AtomicInteger sequence;
@@ -51,38 +51,38 @@ private WhereConditionVisitor(Builder<T> builder) {
5151
}
5252

5353
@Override
54-
public FragmentAndParameters visit(AbstractListValueCondition<T> condition) {
54+
public Optional<FragmentAndParameters> visit(AbstractListValueCondition<T> condition) {
5555
FragmentCollector fc = condition.mapValues(this::toFragmentAndParameters)
5656
.collect(FragmentCollector.collect());
5757

5858
if (fc.isEmpty()) {
59-
return null;
59+
return Optional.empty();
6060
}
6161

6262
return FragmentAndParameters.withFragment(condition.renderCondition(columnName(), fc.fragments()))
6363
.withParameters(fc.parameters())
64-
.build();
64+
.buildOptional();
6565
}
6666

6767
@Override
68-
public FragmentAndParameters visit(AbstractNoValueCondition<T> condition) {
68+
public Optional<FragmentAndParameters> visit(AbstractNoValueCondition<T> condition) {
6969
return FragmentAndParameters.withFragment(condition.renderCondition(columnName()))
70-
.build();
70+
.buildOptional();
7171
}
7272

7373
@Override
74-
public FragmentAndParameters visit(AbstractSingleValueCondition<T> condition) {
74+
public Optional<FragmentAndParameters> visit(AbstractSingleValueCondition<T> condition) {
7575
String mapKey = formatParameterMapKey(sequence.getAndIncrement());
7676
String fragment = condition.renderCondition(columnName(),
7777
getFormattedJdbcPlaceholder(mapKey));
7878

7979
return FragmentAndParameters.withFragment(fragment)
8080
.withParameter(mapKey, condition.value())
81-
.build();
81+
.buildOptional();
8282
}
8383

8484
@Override
85-
public FragmentAndParameters visit(AbstractTwoValueCondition<T> condition) {
85+
public Optional<FragmentAndParameters> visit(AbstractTwoValueCondition<T> condition) {
8686
String mapKey1 = formatParameterMapKey(sequence.getAndIncrement());
8787
String mapKey2 = formatParameterMapKey(sequence.getAndIncrement());
8888
String fragment = condition.renderCondition(columnName(),
@@ -92,12 +92,12 @@ public FragmentAndParameters visit(AbstractTwoValueCondition<T> condition) {
9292
return FragmentAndParameters.withFragment(fragment)
9393
.withParameter(mapKey1, condition.value1())
9494
.withParameter(mapKey2, condition.value2())
95-
.build();
95+
.buildOptional();
9696
}
9797

9898

9999
@Override
100-
public FragmentAndParameters visit(AbstractSubselectCondition<T> condition) {
100+
public Optional<FragmentAndParameters> visit(AbstractSubselectCondition<T> condition) {
101101
SelectStatementProvider selectStatement = SelectRenderer.withSelectModel(condition.selectModel())
102102
.withRenderingStrategy(renderingStrategy)
103103
.withSequence(sequence)
@@ -108,13 +108,13 @@ public FragmentAndParameters visit(AbstractSubselectCondition<T> condition) {
108108

109109
return FragmentAndParameters.withFragment(fragment)
110110
.withParameters(selectStatement.getParameters())
111-
.build();
111+
.buildOptional();
112112
}
113113

114114
@Override
115-
public FragmentAndParameters visit(AbstractColumnComparisonCondition<T> condition) {
115+
public Optional<FragmentAndParameters> visit(AbstractColumnComparisonCondition<T> condition) {
116116
String fragment = condition.renderCondition(columnName(), tableAliasCalculator);
117-
return FragmentAndParameters.withFragment(fragment).build();
117+
return FragmentAndParameters.withFragment(fragment).buildOptional();
118118
}
119119

120120
private FragmentAndParameters toFragmentAndParameters(Object value) {
@@ -170,9 +170,9 @@ public Builder<T> withTableAliasCalculator(TableAliasCalculator tableAliasCalcul
170170
}
171171

172172
public Builder<T> withParameterName(String parameterName) {
173-
parameterPrefix = Optional.ofNullable(parameterName)
174-
.map(pn -> pn + "." + DEFAULT_PARAMETER_PREFIX) //$NON-NLS-1$
175-
.orElse(DEFAULT_PARAMETER_PREFIX);
173+
if (parameterName != null) {
174+
parameterPrefix = parameterName + "." + DEFAULT_PARAMETER_PREFIX; //$NON-NLS-1$
175+
}
176176
return this;
177177
}
178178

‎src/test/java/org/mybatis/dynamic/sql/where/render/RenderedCriterionTest.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,11 @@
2525

2626
public class RenderedCriterionTest {
2727

28-
@Test
29-
public void testNoCriteria() {
30-
Optional<RenderedCriterion> rc = new RenderedCriterion.Builder()
31-
.withConnector(Optional.empty())
32-
.build();
33-
34-
assertThat(rc.isPresent()).isFalse();
35-
}
36-
3728
@Test
3829
public void testSimpleCriteria() {
3930
RenderedCriterion rc = new RenderedCriterion.Builder()
4031
.withConnector(Optional.of("and"))
41-
.withInitialCondition(FragmentAndParameters.withFragment("col1 = :p1").build())
32+
.withInitialCondition(FragmentAndParameters.withFragment("col1 = :p1").buildOptional())
4233
.build()
4334
.get();
4435

0 commit comments

Comments
 (0)