Is it good idea to use Optional.orElseGet to do some logging logic

问题: I want to use Optional for handling null values, the "tricky" part which I cannot think of what is the best way to do - is that I want to do logging if value is null. I can...

问题:

I want to use Optional for handling null values, the "tricky" part which I cannot think of what is the best way to do - is that I want to do logging if value is null. I can achieve that with following code - but it feels awkward.

Lets say code looks like this:

// logLine.getSomeProperty returns Optional<String>

List<LogDetails> logDetails = logLine.getSomeProperty()
    .map(this::extractLogDetails)
    .orElseGet(() -> logError(logLine));
List<LogDetails> extractLogDetails(String s) {
    List<LogDetails> logDetails = new ArrayList<>();
    String sp = "(?:([A-Z0-9]{5,7})-([A-Z0-9]{9})-(.{4}))"; 
    Pattern p = Pattern.compile(sp, Pattern.CASE_INSENSITIVE);
    Matcher m = p.matcher(s);
    while (m.find()) {
        logDetails.add(new LogDetails(m.group(1), m.group(2), m.group(3)));
    }
    return logDetails;
}
List<LogDetails> logError(LogLine logLine) {
    log.error("Error while ... {} ", logLine));
    persistence.setErrorStatus(logLine, FAILED_PARSING);
    return new ArrayList<>();
}

It would do what I want, but I have several "problems" with it.

  • I found it odd, that method called orElseGet is used for logging errors.
  • I could replace orElseGet with orElseThrow and logError there and DO NOT throw anything - which I don't like either.
  • logError method returns List which I don't use and it looks weird to return something from method which should be void.
  • Simply there must be better way
  • Cases where someProperty is not null, but there are no matches - I would like to log as well, but for that I would need another line of code to check if logDetails.size() == 0

回答1:

The orElseGet is not really intended as an error handling mechanism, but as a way to generate a different default value in case the Optional instance is not carrying any.

If you want to check if the Optional is empty explicitly, simply use the Optional.isPresent() check, and do the logError() in that case.

What you need to think first is, if the Optional is empty, what do you want to do? Apart from logging the error, do you want to proceed with an empty list?

If yes then you could have something like this:

List<LogDetails> logDetails = logLine.getSomeProperty()
    .map(this::extractLogDetails)
    .orElseGet(Collections::emptyList);

After which you could do:

if (logDetails.isEmpty()) {
  logError(logline);
}

Alternatively, if you do not want to have an empty list at all, you could keep things at optional level. This way, both cases where the getSomeProperty() is empty or when the generated list is empty are handled in the same way.

Optional<List<LogDetails>> logDetailsOpt = logLine.getSomeProperty() 
                       .map(this::extractLogDetails)
                       .filter(list -> !list.isEmpty());

if (!logDetailsOpt.isPresent()) {
  logError(logLine);
}

In both cases, logError() is not expected to return anything. It is doing what it is intended to do in its name, logging the error.

Rather than trying to overuse the functionality of Optional, try to make your intentions in your code clear. There is more value in readability.


回答2:

Rather than changing result type or logging inside stream you can simply return partitioned Map. Then after obtaining the result, execute log function on the resulting map accordingly.

 Map<Boolean, List<String>> map = Stream.of("a", "aaa", "aaaa")
           ----
                .collect(() -> Collectors.partitioningBy(predicate))
           ----
  • 发表于 2019-01-21 23:24
  • 阅读 ( 300 )
  • 分类:网络文章

条评论

请先 登录 后评论
不写代码的码农
小编

篇文章

作家榜 »

  1. 小编 文章
返回顶部
部分文章转自于网络,若有侵权请联系我们删除