Code review feedback - toOptionalInstance

Author: onobc

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/PropertyMapper.java

@@ -18,6 +18,7 @@
 
 import java.util.NoSuchElementException;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -310,20 +311,18 @@ public <R> R toInstance(Function<T, R> factory) {
 		}
 
 		/**
-		 * Complete the mapping by creating a new instance from the non-filtered value or
-		 * using the specified default instance when the value is filtered.
+		 * Complete the mapping by creating a new instance from the non-filtered value.
 		 * @param <R> the resulting type
 		 * @param factory the factory used to create the instance
-		 * @param defaultInstance the instance to use if the value has been filtered
-		 * @return the instance
+		 * @return the created instance or empty when the value is filtered
 		 */
-		public <R> R toInstance(Function<T, R> factory, R defaultInstance) {
+		public <R> Optional<R> toOptionalInstance(Function<T, R> factory) {
 			Assert.notNull(factory, "Factory must not be null");
 			T value = this.supplier.get();
 			if (!this.predicate.test(value)) {
-				return defaultInstance;
+				return Optional.empty();
 			}
-			return factory.apply(value);
+			return Optional.ofNullable(factory.apply(value));
 		}
 
 		/**

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/PropertyMapperTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.boot.context.properties;
 
+import java.util.Optional;
 import java.util.function.Supplier;
 
 import org.junit.jupiter.api.Assertions;
@@ -209,21 +210,21 @@ void whenWhenValueMatchesShouldSupportChainedCalls() {
 	}
 
 	@Test
-	void whenValueNotFilteredIsInstanceWithDefaultShouldReturnFactoryInstance() {
-		String result = this.map.from("123").when((s) -> s.contains("2")).toInstance(String::new, "foo");
-		assertThat(result).isEqualTo("123");
+	void toOptionalInstanceWithNonFilteredValueShouldReturnFactoryValue() {
+		Optional<String> result = this.map.from("123").whenEqualTo("123").toOptionalInstance(String::new);
+		assertThat(result).isNotEmpty().hasValue("123");
 	}
 
 	@Test
-	void whenValueFilteredIsInstanceWithDefaultShouldReturnDefault() {
-		String result = this.map.from("123").when((s) -> s.contains("x")).toInstance(String::new, "foo");
-		assertThat(result).isEqualTo("foo");
+	void toOptionalInstanceWithNonFilteredValueShouldTolerateNullFactoryValue() {
+		Optional<String> result = this.map.from("123").whenEqualTo("123").toOptionalInstance((s) -> null);
+		assertThat(result).isEmpty();
 	}
 
 	@Test
-	void whenValueFilteredIsInstanceWithNullDefaultShouldReturnNull() {
-		String result = this.map.from("123").when((s) -> s.contains("x")).toInstance(String::new, null);
-		assertThat(result).isNull();
+	void toOptionalInstanceWithFilteredValueShouldReturnEmpty() {
+		Optional<String> result = this.map.from("123").whenEqualTo("foo").toOptionalInstance(String::new);
+		assertThat(result).isEmpty();
 	}
 
 	static class Count<T> implements Supplier<T> {