Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)

Subject: Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)
From: Rob Stewart (rstewart_at_[hidden])
Date: 2018-02-03 22:32:32

Herewith is my review of the Outcome library.

Design

The design of the library is complex. I'm not sure that a simpler design would be sufficient since it tries to do a lot of things. But therein lies the rub: it smacks of trying to be too many things for too many people, which requires it to be complex.

There are two principle templates, but potentially many different types using them. There are many policies and function templates to tweak the behavior of the library. The latter is somewhat disturbing as one must always include all moving parts for a particular outcome or result type in order to get all of the customizations. In practice, everything is likely to be defined in a single header, so that may not be a problem, but it seems complicated. What's more, some customizations are done using free functions, found via ADL, and others via policy classes. That's discomfiting, at least.

Implementation

I did not have time to examine the implementation.

Documentation

The docs are thorough, though in need of lots of editing. The tutorial is very long and often overly complex. A more typical approach is to use a tutorial as an introduction and to use a separate section of the documentation for more advanced topics and examples.

I generated a lot of comments while working my way through the documentation. I've noted that some examples are poor choices for justifying or illustrating the value of the library. I'll forward them via email rather than include them here as they are extensive (and I didn't even get through the entirety of the documentation).

Usefulness

I did not try to use the library, but I did consider it's usefulness in libraries I've written or used that have the traditional error_code/exception overloads. At the API level, a library writer can avoid massive overloading to account for throwing and non-throwing versions of each operation. However, from the perspective of the library user, that work is irrelevant. Being able to write "if (auto retval = foo()) { use foo.value() } else { react to error }" can make for a newly idiomatic, streamlined style when using such libraries (as opposed to writing "std::error_code error; auto value = foo(error); if (error) { use value } else { use error }"). However, when using the exception throwing overloads, the code gets worse: "foo().value()" vs. "foo()". IME, one makes relatively few calls to libraries of this sort, in any given block of code, so the relative verbosity is probably insignificant.

That result<T,EC> is declared with [[nodiscard]] can be helpful. It means the return value of a function returning a result cannot be ignored. Unfortunately, the compiler doesn't require that the programmer do anything with the result except save it. (A warning might alert the programmer to do something with such a variable, but there's no enforcement.) The same is not documented for outcome, however. Thus, when using outcome, because one might need to convey an exception to the caller, there is no compiler help to ensure the programmer doesn't ignore the return value. Even with that fixed, there's still no library help to ensure the programmer inspects the return value.

As might be clear, I'm torn as to whether this library would be useful to me.

How much effort?

I spent many hours going through the documentation and considering the usefulness of the library.

Problem Domain Knowledge

Having written and used lots of libraries over the years, I've considered and reconsidered how best to convey error information to callers. I normally don't have to worry about latency issues, but neither do I have the luxury of adding unnecessary overhead to library code. Thus, I consider myself well qualified in this domain.

Summary

I find that I cannot vote to accept the library into Boost. I am not rejecting it, but I don't expect to make use of the library, so I can't bring myself to vote for its inclusion in Boost. The library seems sound, it seems like it could be useful, and it is reasonably well documented. Inclusion in Boost could provide important exposure that could shape the library before it is proposed for standardization, which Niall intends, but I'm not convinced that's reason enough to accept it into Boost.
--
Rob
(Sent from my portable computation device.)