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

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. It is overwhelming to the uninitiated though, so I can see how it might appear to be complex. I would argue that being overwhelmed with the possibilities would be a characteristic of any vocabulary library. 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. I agree that the granularity of the header includes needs to be improved so people can include and not drag anything but basic_result. That's already logged, and will be done.

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

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.

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.

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.

Thank you for your review. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/