rdblue commented on a change in pull request #1531:
URL: https://github.com/apache/iceberg/pull/1531#discussion_r499873477
##########
File path: core/src/main/java/org/apache/iceberg/LocationProviders.java
##########
@@ -45,6 +50,53 @@ public static LocationProvider locationsFor(String location,
Map<String, String>
}
}
+ private static LocationProvider dynamicallyLoadLocationProvider(String
location, Map<String, String> properties) {
+ String impl = properties.get(TableProperties.WRITE_LOCATION_PROVIDER_IMPL);
+ Optional<DynConstructors.Ctor<LocationProvider>> noArgConstructor =
findConstructor(() ->
+ DynConstructors.builder(LocationProvider.class)
+ .impl(impl).build()
+ );
+ Optional<DynConstructors.Ctor<LocationProvider>> twoArgConstructor =
Optional.empty();
Review comment:
When the arguments passed to one constructor are a prefix of the args
passed to the other, there is no need to use a different builder or call
because the arguments actually passed are limited by the number of parameters
defined by the constructor. That makes it possible to pass both arguments to
either one, and any extras are ignored.
That helps simplify cases like this because you don't need the extra
optional logic:
```java
ctor = DynConstructors.builder(LocationProvider.class)
.impl(impl, String.class, Map.class)
.impl(impl) // fall back to no-arg constructor
.build()
return ctor.newInstance(location, properties);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
