Является ли этот пример использования Optionals в Java 8 правильным? Как бы вы переписали это? - PullRequest
Новая
       4

Является ли этот пример использования Optionals в Java 8 правильным? Как бы вы переписали это?

0 голосов
/ 11 ноября 2018

Я начал использовать 8 опций Java.и я хотел бы поделиться этим методом, это пример "кода пахнет", и я хотел бы переписать его, используя java 8 и optionlas и функциональный (декларативный) стиль, поэтому мне интересно узнать ваше мнение об этом.Давайте рассмотрим этот метод: 

  public boolean isTokenValid(String userAgent, Optional<String> apiKey) {
    LOGGER.info(String.format("userAgent : %s, Key ?: %s", userAgent, apiKey.isPresent()));
    if ("ALFA".equalsIgnoreCase(userAgent)){
        return (apiKey != null && apiKey.isPresent()
                && ispropertyExists(ALFA_TYPE, apiKey.get()));
    }
    else {
        return (apiKey != null && apiKey.isPresent()
                && ispropertyExists(BETA_TYPE, apiKey.get()));
    }
}

Где «ispropertyExists» возвращает логический тип, а «ALFA_TYPE» и «OMEGA_TYPE» являются константами перечислений.Ниже приведен способ, которым я переписал этот метод, чтобы улучшить читаемость и практиковать стиль функционального мышления.Я добавил комментарии, чтобы объяснить мои мысли и причины, по которым я это сделал, и я ценю ваше мнение и примеры ваших способов, если вы считаете, что можете улучшить его. 

    /**
 * We should not pass Optionals as a parameters to the methods. We
 * should use Optionals only for return value when we are not sure if value will
 * be presented at the end of the calculations or not.
 */
public boolean isTokenValid(String userAgent, String apiKey) {
    LOGGER.info(String.format("userAgent : %s, Key ?: %s", userAgent, apiKey));

    /**
     * If apiKey is null then it is incorrect. And execution will stop after
     * Optional.ofNullable(apiKey), since monad will be having null value. If apiKey
     * is not null then we still want to filter out empty strings. If after filter
     * there will be no value, then execution will stop.
     * If we have some value for apiKey then it is ok and we map the monad to the
     * userAgent value to proceed the chain of calls on monad.
     */
    Optional<String> userAgentOptional = Optional.ofNullable(apiKey).filter(StringUtils::isNotBlank)
            .map(ak -> userAgent);

    /**
     * We map "userAgent" value to boolean (if it is a alfa or not). Then
     * we map that boolean to boolean value which represents security check in db
     * itself.
     */
    Optional<Boolean> isValid = userAgentOptional.map(ua -> "ALFA".equalsIgnoreCase(ua))
            .map(isAlfa -> isAlfa ? ispropertyExists(ALFA_TYPE, apiKey)
                    : ispropertyExists(BETA_TYPE, apiKey));

    /**
     * And all in all we get value from our optional boolean. If "somehow" it is
     * ended up to be empty, then we retrun "false", if it is not empty, then the
     * value will itself be returned.
     */
    return isValid.orElse(false);
}

Спасибо.

Ответы [ 3 ]

0 голосов
/ 11 ноября 2018

ИМХО, вы переусердствуете и слишком усложняете это. Я согласен, что в общем случае мы не должны передавать Optional в качестве параметра методу. Если вы не можете требовать, чтобы переданный apiKey был ненулевым, я бы предложил: 

public boolean isTokenValid(String userAgent, String apiKey) {
    LOGGER.info(String.format("userAgent : %s, Key : %s", userAgent, apiKey));
    if (apiKey == null || apiKey.isEmpty()) {
        return false;
    }
    return ispropertyExists(
            userAgent.equalsIgnoreCase("ALFA") ? ALFA_TYPE : BETA_TYPE, apiKey);
}

Я бы нашел это значительно проще. Для вашего случая нет необходимости использовать Optional.

0 голосов
/ 12 ноября 2018

Я бы объединил все операции в одном цепочечном операторе и возвратил бы результат, избегая ненужных Optional переменных. 

 return Optional.ofNullable(apiKey)
                .filter(StringUtils::isNotBlank)
                .map(ak -> userAgent)
                .map("ALFA"::equalsIgnoreCase)
                .map(isAlfa -> isAlfa ? ALFA_TYPE : BETA_TYPE)
                .map(type -> ispropertyExists(type, apiKey))
                .orElse(false);
0 голосов
/ 11 ноября 2018

Если вы любите функциональный стиль, сначала постарайтесь не использовать null или, по крайней мере, не передавайте null.

Но если вы должны использовать null, вот мой код для вас: 

public boolean isTokenValid(String userAgent, String apiKey) {
    final Enum type = "ALFA".equalsIgnoreCase(userAgent) ? ALFA_TYPE : BETA_TYPE;
    return
        Optional.ofNullable(apiKey)
        .filter(ak -> ispropertyExists(type, ak))
        .isPresent();
}

PS: функциональный стиль не означает попытки связать все воедино и избежать временных значений. Скорее речь идет об использовании чистых функций и неизменяемых данных . Независимо от стиля нашей целью является написание читаемого и разумного кода. Чистые функции и неизменные данные очень подходят для этой цели.

...