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-06 00:29:09

On February 3, 2018 8:41:33 PM EST, Niall Douglas via Boost <boost_at_[hidden]> 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<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.
> 
> 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.)