vendredi 28 juillet 2017

Reducing if else branches and cleaning code using chain of functions, does it make sense

I am working on legacy code that has lots of if/else. I can use conditional decomposition and/or guard statements etc. to clean it. Another approach which can be used is to create functions out of branches and execute these functions as chain and exit with first response. I am wondering if using function chain is an overkill? Any views?

My chain looks something like below. This is a simplified example.

public class RequestStateHandler {
    private final List<Function<RequestStateInfo, Optional<Response>>> handlers = new ArrayList<>();

    public RequestStateHandler() {
        handlers.add(requestStateInfo -> xStateValidator(requestStateInfo));
        handlers.add(requestStateInfo -> yStateValidator(requestStateInfo));
        handlers.add(requestStateInfo -> zStateValidator(requestStateInfo));
    }

    public Response getResponse(RequestStateInfo requestStateInfo) {
        try {
            for (final Function<RequestStateInfo, Optional<Response>> handler : handlers) {
                final Optional<Response> response = handler.apply(requestStateInfo);

                if (response.isPresent()) {
                    return response.get();
                }
            }
        } catch (Exception e) {
            LOGGER.error("Some log", e);
        }

        return getDefaultResponse(requestStateInfo);
    }

    private Optional<Response> xStateValidator(RequestStateInfo requestStateInfo) {
        final A a = requestStateInfo.getA();
        if (a.isABC()) {
            return Optional.of(requestStateInfo.getX());
        }

        return Optional.empty();
    }

    private Optional<Response> yStateValidator(RequestStateInfo requestStateInfo) {
        if (requestStateInfo.isXYZ()) {
            final Response response = requestStateInfo.getX();
            final A a = response.getA();
            if (a.isSomeOtherTest()) {
                return Optional.of(response);
            }
        }

        return Optional.empty();
    }

    private Optional<Response> zStateValidator(RequestStateInfo requestStateInfo) {
        final Response response = requestStateInfo.getX();
        final A a = response.getA();
        if (a.isSomeAnotherTest()) {
            return Optional.of(response);
        }

        return Optional.of(getDefaultResponse(requestStateInfo));
    }

    private Response getDefaultResponse(RequestStateInfo requestStateInfo) {
        return new Response(A.create(requestStateInfo.getY()), null, null, null);
    }
}

I thought of creating a reusable executor so that we can have only business logic here. To me it looks much cleaner but it obviously add a dependency to execute the functions in order. Something like chain of responsibility, I would say. What are your thoughts?

A generic executor could be something like:

import org.apache.commons.lang3.Validate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;

public class ChainedExecutor<T, R> {
    private final static Logger LOGGER = LoggerFactory.getLogger(ChainedExecutor.class);
    public final List<Function<T,Optional<R>>> handlers;
    private R defaultResponse;

    private ChainedExecutor() {
        this.handlers = new ArrayList<>();
    }

    public R execute(T request) {
        return execute(request, Optional.empty(), Optional.empty());
    }

    public R execute(T request, Function<T, R> defaultSupplier) {
        return execute(request, Optional.of(defaultSupplier), Optional.empty());
    }

    public R execute(T request, Consumer<Exception> exceptionHandler) {
        return execute(request, Optional.empty() ,Optional.of(exceptionHandler));
    }

    public R execute(T request, Function<T, R> defaultHandler, Consumer<Exception> exceptionHandler) {
        return execute(request, Optional.of(defaultHandler), Optional.of(exceptionHandler));
    }

    public R execute(T request, Supplier<R> defaultSupplier) {
        final Function<T, R> defaultHandler = (input) -> defaultSupplier.get();
        return execute(request, Optional.of(defaultHandler), Optional.empty());
    }

    private R execute(T request, Optional<Function<T, R>> defaultHandler, Optional<Consumer<Exception>> exceptionHandler) {
        Optional<R> response;
        try {
            for (final Function<T,Optional<R>> handler: handlers) {
                response = handler.apply(request);

                if (response.isPresent()) {
                    return response.get();
                }
            }
        }
        catch (Exception exception) {
            handleOrLog(exceptionHandler, exception);
        }

        return getDefaultResponse(defaultHandler, request);
    }

    private void handleOrLog(Optional<Consumer<Exception>> exceptionHandler, Exception exception) {
        if (exceptionHandler.isPresent()) {
            exceptionHandler.get().accept(exception);
            return;
        }
        LOGGER.error("Unhandled exception occurred while executing handler chain. This most probably is a developer error.");
    }

    private R getDefaultResponse(Optional<Function<T, R>> defaultHandler, T request) {
        if (defaultHandler.isPresent()) {
            return defaultHandler.get().apply(request);
        }
        return getDefaultResponse();
    }

    public R getDefaultResponse() {
        return defaultResponse;
    }

    public static class Builder<T, R> {
        private final ChainedExecutor<T, R> chainedExecutor = new ChainedExecutor<>();

        public Builder(List<Function<T,Optional<R>>> handlers) {
            Validate.notNull(handlers);
            for (final Function<T, Optional<R>> handler: handlers) {
                Validate.notNull(handler);
                handlers.add(handler);
            }
        }

        public Builder() {}

        public ChainedExecutor.Builder<T, R> withHandler(Function<T, Optional<R>> handler) {
            Validate.notNull(handler);
            chainedExecutor.handlers.add(handler);
            return this;
        }

        public ChainedExecutor.Builder<T, R> withDefaultResponse(R defaultResponse) {
            Validate.notNull(defaultResponse);
            chainedExecutor.defaultResponse = defaultResponse;
            return this;
        }

        public ChainedExecutor<T, R> build() {
            Validate.isTrue(!chainedExecutor.handlers.isEmpty());
            return chainedExecutor;
        }
    }
}

Aucun commentaire:

Enregistrer un commentaire