Add unwrapErr for Results with non-void Error kinds#4
Conversation
Remove template params that prevent compilation where copy is necessary
| typename std::enable_if< | ||
| !std::is_same<F, void>::value, | ||
| F | ||
| >::type |
There was a problem hiding this comment.
I don't think that you need to SFINAE here. I have a static_assert that prevents from having a void error-type: https://github.com/oktal/result/blob/master/result.h#L706
There was a problem hiding this comment.
Why isn't a void error type allowed? I can see that not being too helpful in production code, but having a void error type could still be useful for prototyping or simple applications. It's also allowed in Rust (as Result<T, ()>).
There was a problem hiding this comment.
Right, but I think it will be a little bit painful to handle.
Currently, you might want to try defining a unit type like struct Unit { } and use it as a void error type, what do you think ?
There was a problem hiding this comment.
That works. I talked about it with a friend and he made the exact same suggestion. I've made and pushed the changes to this PR.
|
I think that might work short term. It's not something people will expect when working with the library, but we could work to improve support for void error types, right? As a later feature. |
oktal
left a comment
oktal
left a comment
There was a problem hiding this comment.
Alright, when the return type of unwrapErr will be fixed, the PR will be ready to be merged in.
| std::terminate(); | ||
| } | ||
|
|
||
| unwrapErr() const { |
There was a problem hiding this comment.
I think you removed the void return type by inadvertence
|
Woops, I must have hit something just as I was exiting the file. I fixed it and retested it to make sure it could be called as well. |
|
Great, looks good to me, thanks for the contribution 👍 |
2 participants
Added unwrapErr to the API of Result.