29 May
2020
29 May
'20
8:26 a.m.
Disclaimer: This is from someone not using Boost.Exception, Expected or any other resultlibrary and judging from the documentation. So this might serve in evaluating how well the lib/documentation fares to someone thinking to adopt this. > - What is your evaluation of the design? There is a mixture of lambdas and macros which I find a bit off-putting on first sight. However it seems they solve the problem well in the constraints of the language. I find the default for `is_e_type` being true for any X having X::value a bit broad. This basically applies to all strong types so it feels like an "E-Type" could be anything. So I don't understand why the distinction into E-types and non-E-types is made. Maybe improve on `remote_handle_all` in 2 ways: Create the lambda taking `leaf::error_info const&` in that function to remove the boilerplate at the user call site. Given that rename it to e.g. `create_remote_handler` or so. I find the current interface confusing and the documentation calls it "unintuitive", so I guess this should be improved on before release. Given that: It seems `remote_try_handle_all` can be folded into the regular `try_handle_all`. So `try_handle_all` takes a list of handlers where one could be the aggregated handler defined by `create_remote_handler` (then maybe `create_aggregate_handler`?) I'm interested in knowing how `try_handle_all` implements "if the user fails to supply at least one handler that will match any error, the result is a compile error". If arbitrary functions can be called throwing any error type, how could that work? Is it again required to use a catch-all clause? > - What is your evaluation of the documentation? Good overall, but the details are to detailed for an introduction and to coarse for understanding it. Examples by header: - "Using Exception Handling": virtual inheritance is mentioned but the reasoning is not clear to me. I have never used or seen it used in that context and always strive to avoid it due to the issues with handling such an object (starting at writing ctors) - "Accumulation": shows how to call leaf::accumulate but I don't see what it use is. "Many different files" are mentioned but the operation only takes a single file. And based on everything before a single error stops execution (like exceptions) - "E-types": I didn't got what an E-type is exactly. It starts with "values" you can add to "errors" or "exceptions" but seemingly exceptions itself are "E-types"? See also above. I also didn't see the enums being mentioned here that are used in other places. This would be a great addition - "Loading": It starts how values are put into a `context`. I feel that a detailed introduction how a context works and is used/accessed prior to this would be useful. My point is: The `leaf::load` call is somewhere down the chain not knowing anything about the `try_handle_*` call and hence the context. How does it know about it? Or what to put there? So maybe start a first paragraph solely on the context and its access - "error_id": It is unclear to me what it's use is. Especially as something unique in the program in the context of threads means some kind of synchronization which will be paid for even if unused. - "defer": I'd like to know when the lambda is not called. The documentation says it is called in the dtor, but later: "function passed to defer, if invoked," - "Working with Disparate Error Types": To me it is unclear what the message of this part is. Maybe it can be shortened. - "Lua": Code example is missing the namespace for `augment_id` and there is likely a typo "aughent" Wording at some places might be improved. - "With LEAF, users can efficiently associate with errors or with exceptions any number of values that pertain to a failure", for me this is hard to understand. - "Let’s say we want to build a record of file locations a given error passes through on its way to be handled", is something missing here? As it is a C++11 library I'd suggest to not use plain enums in the documentation. E.g. `enum do_work_error_code` in the lua example with such generic names. This might lead beginners into C&P the code. > - What is your evaluation of the potential usefulness of the library? It seems to solve the same problem as other error handling libraries but in a new way. Performance-wise it seems to be comparable. I'd suggest to evaluate the cases where it lacks behind and whether those can be improved Giving the intention to unify error handling (or try to make it more uniform) I'm wondering if this could be standardized. Given that Boost is often a "playground for future C++ standards" I feel this should be evaluated whether this is possible at all. Of course I'd love to see language support avoiding lambdas and macros, but I'm not optimistic regarding that. I'd also like to see a comparison with exceptions especially in cases where the usage of result inhibits (N)RVO (even more for C++17 and up). This is my main critic point about result type of error handling as that variant type creates less efficient code for the happy path. All in all I think it is useful to experiment with a new approach and as such fits Boost > - Did you try to use the library? With which compiler(s)? Did you > have any problems? Didn't try > - How much effort did you put into your evaluation? A glance? A quick > reading? In-depth study? In-depth reading of documentation only. > - Are you knowledgeable about the problem domain? Not really.