On 30/05/2017 22:38, Andrzej Krzemienski via Boost wrote:
2017-05-30 16:58 GMT+02:00 Niall Douglas via Boost
: With regard to the previous high level summary at http://boost.2283326.n4.nabble.com/outcome-High-level- summary-of-review-feedback-accepted-so-far-tp4694767.html, I have made only these changes to the changes in that summary:
Thanks Niall, for the summary. But now, after these changes, the review I am still writing is useless, as it applies to a different library. Maybe we should schedule another review?
As I have said in a number of posts previously, I haven't been convinced to change much yet apart from drop some member functions for the runtime-checked empty-capable editions which are the ones presented for review. The behaviours of those are still being actively discussed e.g. should exception() synthesize an exception_ptr from an errored state? And even in the added varieties, it's not like I am writing any new source code. The statically checked varieties just leave out the runtime checks, otherwise they're literally identical consisting of identical implementation. The non-empty varieties just leave out the empty state. It's the same library as presented.
1. I have been persuaded to use longer more appropriate naming for result<T> and outcome<T>, so now the typedefed varieties with implicit conversions to their empty-capable form indicated by "=>" are:
- static_checked_outcome<T> => static_checked_optional_outcome<T> static_checked_result<T> => static_checked_optional_result<T>
No, no. I strongly suggest to provide only one outcome, with empty state if needed, and both narrow and wide contracts. We have learned to live with such types in C++, even if they are far from perfect.
I am absolutely sold on the empty-capable vs non-empty-capable being represented in the type system. Makes a big difference to public API contracts. I may choose to drop the typedef for the statically checked varieties, but the underlying non-public template is capable of all. I'll see what Charley's report recommends before deciding.
3. Under the assumption that error_code_extended's use of a static global ring buffer would be controversial in this review, I hacked a quick and dirty solution expecting to have to remove it. Now we know that few disapprove, a more serious implementation will be needed. https://github.com/ned14/boost.outcome/issues/52
I am not sure of that. It was my impression that many potential reviewers were put off by documentation and legal/structural issues, and stopped with their review at that point. Once you have move thes obstacles out of the way, you will likely draw more reviewers, and who knows what they have to say about the ring buffer.
Dunno, I got quite a few detailed questions on it, and people seemed pleased with the design rationale once they'd thought about it. Anyway, I intend to do something better than the current hack. Certainly with more storage, anyway. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/