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

On February 3, 2018 8:41:33 PM EST, Niall Douglas via Boost wrote:

...

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.

It really isn't complex, v2 is a marked simplification over v1 as the previous review requested.

That it is less complex than v1 does not mean it isn't complex.

It is overwhelming to the uninitiated though, so I can see how it might appear to be complex.

Surely that is a sign of complexity. That doesn't mean it is unnecessary complexity, however.

I would argue that being overwhelmed with the possibilities would be a characteristic of any vocabulary library.

The same cannot be said for shared_ptr, though it probably could be said of chrono.

Outcome is not a niche purpose library like we usually review here in recent years. Its scope of application has a lot of knock on consequences. Most of the negative feedback posted in this review so far is clearly grounded in concerns over effects on the ecosystem, because if this thing enters Boost and it's broken, then all the stuff built with it is also going to be broken. Such is the burden of vocabulary libraries and language features.

...

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.

The policy classes seem to be much more scary than I had anticipated. Honestly, try implementing your own. You'll be done within ten minutes. They're very simple.

Perhaps the solution is basic_result and basic_outcome, with result and outcome as special, simplifying cases. That way, most can use the normal, simpler templates, but those needing all of the knobs and levers for a custom use case can use the basic_* types.

...

What's more, some customizations are done using free functions, found via ADL, and others via policy classes. That's discomfiting, at least.

The cause is the lack of C++ language support for something like https://github.com/ldionne/dyno. A key goal of Outcome is *non-intrusive* rule setting of how third party code ought to interact without requiring modification of third party source code. It unavoidably, given the limitations of the language, relies heavily on ADL customisation points. I will say that where I was able to avoid ADL, I did lift rule setting as much as possible into trait specialisation in its own namespaces so it is clearly delineated and has minimum chance of unexpected interaction.

It still seems like one or the other would be better.

I agree that what has resulted looks like a dog's breakfast, and again it's overwhelming. I could have made everything ADL for purity, but I find ADL worrying. Too much chance of unexpected surprise. Best minimised to the absolute minimum in my book. But the consequence then is that dog's breakfast. I'm not sure what else one can do, other than ditch the non-intrusive interoperation support. And if reviewers did want that gone, it could be removed and that would make things appear much cleaner.

I'm not sure what you're implying as an alternative.

The complexity would then, of course, merely be pushed onto the end user having to manually unpack and repack Expected into Outcome etc. Maybe that's better. I had hoped for more feedback from reviewers on whether it would be better or not.

I have no idea to what you refer, so I can't compare the two.

...

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).

I look forward to receiving those. But please be aware that I have written a tutorial for Outcome five full times now. I am beginning to realise that a tutorial which makes even a majority happy is not possible given my very finite resources. I need to draw a line at some point for my sanity, there needs to be a life which is not working forever more writing Outcome documentation.

Good documentation can be difficult, and writing it can be all but impossible for many.

...

That result 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.

The only solution to this that I am aware of is if outcome and result's destructors throw. I felt that unwise, but it could be implemented if reviewers felt it beneficial.

For those that actually run debug builds, an assertion would work nicely. -- Rob (Sent from my portable computation device.)