2017-05-19 0:49 GMT+02:00 Niall Douglas via Boost <boost@lists.boost.org>:
My personal preference on this is that if you call `o.error()` before you have confirmed you actually have an error, you are doing something wrong. I would classify such situation as undefined behavior. But your choice fits into the scope of undefined behaviour: if program can do anything, it might as well return a default-constructed error_code.
Ah but remember that outcome<T> and result<T> both guarantee that E will be a type meeting the std::error_code contract.
Ok, I remember, both error_code and exception_ptr are default-constructible. It is not immediately clear to me that a default-constructed error_code represents a no-error condition. In the blog post by Chris Kohlhoff you refer to, he uses the following enum for representing http error conditions: ``` enum class http_error { continue_request = 100, switching_protocols = 101, ok = 200, ... gateway_timeout = 504, version_not_supported = 505 }; ``` Which implies that numeric value 200 means no-error and a value initialized `http_error` is meaningless. Maybe this does not affect the value of a default-constructed `std::error_code`, but surely it adds to the confusion.
Therefore calling o.error() on a valued outcome returning a default constructed (null) error code is exactly correct: we return there is no error.
Only under some definition of "correct". By correct, you probably mean "not invoking UB" and "being compliant with your specification", but it is not intuitive at all that a function should return this or that when invoked in a context that is most likely a programmer's bug.
No undefined behaviour needed, and you can write your code using Outcome with the hard assumption that o.error() will always return an accurate view of the current state. No need to check .has_error(), or anything like it.
Modulo this situation with `http_error::ok == 200`. But with this you are also saying, the library provides two ways for checking if you have an error: o.has_error(); // option 1 o.error() == std::error_code{}; // option 2 And by describing clear semantics for option 2, you are saying, it is equally fine to use option 2 for checking if we have an error. This encourages the usage of option 2, but I would not want my colleague programmers to start using this syntax, because I then cannot tell proper usages from inadvertent omissions. And reading the value of `error()` without having confirmed that some error occurred is almost surely a bug, even if you can assign a well defined semantics in `boost::outcome` for it.
On the other hand, you do loose something when you chose something else than undefined behavior: the potential for such user bugs to be detected by static analyzers.
No static analyser that I am aware of can diagnose when someone is causing UB in std::optional<T>. Writing one which didn't spew false positives would be hard, even if the entire program were visible. After all, maybe the user is _intentionally_ doing the reinterpret_cast by effectively treating the common state as a union?
Challenge accepted.
Remember, expected<std::error_code, std::error_code> is legal, though Outcome's Expected doesn't allow it.
But I do not understand why in my program I would want to write:
``` if (o == tribool::unknown) {} ```
How is that better ina any way from `o.is_empty()`?
I don't want to talk about this much as it's outside the scope of the current review, but in functional programming using Outcome the tribool can help make the logic much clearer. A common pattern I've used personally is for true and false to select which branch of lambdas to call next, and empty means to terminate processing immediately with no further functions called.
I should emphasise that those extensions are disabled in the presented library, and are out of scope for this review.
I am fine with this scope disabled. However, `tribool` is still in scope, so I consider it valid to ask about its usefulness (especially given that all contexts in which it would prove useful is disabled). Maybe `tribool` should be also removed from the scope? Regards, &rzej;