On Sat, Jan 13, 2018 at 6:09 AM, Andrey Semashev via Boost < boost@lists.boost.org> wrote:
I appreciate that the special meaning of 0 may be undesirable, but having a virtual function call overhead whenever one wants to test if the error_code has a success value seems a too high price for that. I don't want to be paying that price in my Linux programs, sorry.
My understanding is that the current
design explicitly enables many-to-many comparison of ("error-")values across error domains (error categories). Specifically, the implication is that there are "many" success-values; or "many" success-values specific to a given application-specific context. We've already had recent threads discussing how to handle the several "HTTP 2xx" return-values as all indicating "success", but none of them are of value "zero". (Answer: Case out the 'enum' values, or compare to a generic 'success' enum-value designed for that purpose.)
So, we can discuss the "trap-value" of zero as meaning something, but it seems real-world algorithms must handle these multiple success-values (whether or not we want to expect a 'zero' as meaning something in particular).
If we concede that many 'success' values exist, the class of 'std::error_code' values becomes:
(a) empty, (e.g., default-ctor) (b) success (many possible values, context-determined) (c) error (many possible values, context-determined)
I do not view `error_code` as an object that can have an "empty" state.
From my view it is always populated with some code, which happens to be
0 by default. That value also happens to indicate "success" in many
domains, with other values indicating various degrees of failure.
So I believe, only (b) and (c) are valid choices. For (a) you need
`optional
IMHO, the 'if(ec)...' is a problematic construct if it is to imply some form of "success". It seems more suited to imply "no-value-populated"; but I think that is a very limited test that probably isn't very helpful in the real-world: If the caller receives the value from some implementation, the caller can trust it to represent something and doesn't care about the distinction between "empty" and "not-empty", but only between "success" and "not-success".
The "if (err)" test makes very much sense if you just want to know if things are ok, whatever that means. In terms of HTTP errors, that would mean testing for a 2xx status code. And that usage seems to be the most frequent in my experience. If you want to distinguish between different specific results then you would probably test against the specific status codes, which would require the caller to be familiar with the domain of the error code. That is also ok and `error_code` allows that usage as well, but naturally I always try to isolate the problem domain in the component that reports the error so that the caller can operate on a more abstract level. The problem is that `error_code` does not allow for multiple "success" values and adding that support would penalize the cases when there is only one "success" value (which I still think are much more frequent). As much as I would like `error_code` to support multiple success values, I don't think it is worth the performance penalty of a virtual function call. Personally, I would go one of the following routes: - Map "success" values (e.g. all 2xx codes in case of HTTP) to 0 when `error_code` is constructed. Use an additional channel (external to `error_code`) to obtain the exact domain-specific error code, if one needs it. - Do not map success values to 0 but also never use contextual conversion to bool. Always test against the specific error/success codes. - Use a completely different type to store error codes (probably, as simple as an enum) and leave `error_code` alone. Use `error_code` only in domains where its design fits and use other tools where it doesn't. The last option would probably be my preferred one, although it leaks the problem domain to the caller. As an idea, maybe it would be better to introduce a more generic version of `error_code`, and express `error_code` through that class. Something along these lines: template< typename IsSuccessCodeFun > class basic_error_code { // All error_code implementation here public: explicit operator bool() const { return IsSuccessCodeFun{}(value()); } }; typedef basic_error_code< is_zero > error_code; typedef basic_error_code< is_2xx > http_error_code;