TeslaCN commented on a change in pull request #1567:
URL:
https://github.com/apache/shardingsphere-elasticjob/pull/1567#discussion_r507024721
##########
File path:
elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/handler/threadpool/JobExecutorServiceHandlerFactory.java
##########
@@ -42,8 +43,11 @@
*/
public static JobExecutorServiceHandler getHandler(final String type) {
if (Strings.isNullOrEmpty(type)) {
- return
ElasticJobServiceLoader.getCachedInstance(JobExecutorServiceHandler.class,
DEFAULT_HANDLER);
+ return
ElasticJobServiceLoader.getCachedTypedServiceInstance(JobExecutorServiceHandler.class,
DEFAULT_HANDLER)
Review comment:
Is it necessary to check if default handler present? Why could this
happen?
##########
File path:
elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/spi/ElasticJobServiceLoader.java
##########
@@ -106,33 +61,42 @@ private static Object newServiceInstance(final Class<?>
clazz) {
/**
* Get cached instance.
*
- * @param typedService service type
+ * @param typedServiceInterface typed service interface
* @param type specific service type
* @param <T> specific type of service
* @return cached service instance
*/
- public static <T extends TypedSPI> T getCachedInstance(final Class<T>
typedService, final String type) {
- T instance = TYPED_SERVICES.containsKey(typedService) ? (T)
TYPED_SERVICES.get(typedService).get(type) : null;
+ public static <T extends TypedSPI> Optional<T>
getCachedTypedServiceInstance(final Class<T> typedServiceInterface, final
String type) {
+ T instance = TYPED_SERVICES.containsKey(typedServiceInterface) ? (T)
TYPED_SERVICES.get(typedServiceInterface).get(type) : null;
Review comment:
Consider making good use of `Optional#map` if you are going to use
`Optional`.
##########
File path:
elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/spi/ElasticJobServiceLoader.java
##########
@@ -106,33 +61,42 @@ private static Object newServiceInstance(final Class<?>
clazz) {
/**
* Get cached instance.
*
- * @param typedService service type
+ * @param typedServiceInterface typed service interface
* @param type specific service type
* @param <T> specific type of service
* @return cached service instance
*/
- public static <T extends TypedSPI> T getCachedInstance(final Class<T>
typedService, final String type) {
- T instance = TYPED_SERVICES.containsKey(typedService) ? (T)
TYPED_SERVICES.get(typedService).get(type) : null;
+ public static <T extends TypedSPI> Optional<T>
getCachedTypedServiceInstance(final Class<T> typedServiceInterface, final
String type) {
+ T instance = TYPED_SERVICES.containsKey(typedServiceInterface) ? (T)
TYPED_SERVICES.get(typedServiceInterface).get(type) : null;
if (null == instance) {
- throw new JobConfigurationException("Cannot find a cached typed
service instance by the interface: @" + typedService.getName() + "and type: " +
type);
+ return Optional.empty();
}
- return instance;
+ return Optional.of(instance);
}
/**
* New typed instance.
*
- * @param typedService service type
+ * @param typedServiceInterface typed service interface
* @param type specific service type
* @param <T> specific type of service
* @return specific typed service instance
*/
- public static <T extends TypedSPI> T newTypedServiceInstance(final
Class<T> typedService, final String type) {
- Class<?> instanceClass =
TYPED_SERVICE_CLASSES.containsKey(typedService) ?
TYPED_SERVICE_CLASSES.get(typedService).get(type) : null;
+ public static <T extends TypedSPI> Optional<T>
newTypedServiceInstance(final Class<T> typedServiceInterface, final String
type) {
+ Class<?> instanceClass =
TYPED_SERVICE_CLASSES.containsKey(typedServiceInterface) ?
TYPED_SERVICE_CLASSES.get(typedServiceInterface).get(type) : null;
Review comment:
Consider making good use of `Optional#map` if you are going to use
`Optional`.
----------------------------------------------------------------
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]
