Add Result<Value, Error>#19982
Conversation
stdlib/public/core/Result.swift
Outdated
There was a problem hiding this comment.
Since value and error are both calculated, would it not be more performant to have a switch on lhs and rhs and perform the equality tests there?
There was a problem hiding this comment.
It'd be good to have a solid answer one way or another.
stdlib/public/core/Result.swift
Outdated
There was a problem hiding this comment.
Similar to my above comment, would it not be better to do a switch on self here? Of course one of the cases would need to add some extra bit to the hasher in order to ensure that the hashes are different when the Error type and the Value are the same and the values are equal...
There was a problem hiding this comment.
I'm not sure. If it's more efficient, then I'll make the change, but can I annotate the properties to regain the efficiency?
DevAndArtist
left a comment
There was a problem hiding this comment.
The stdlib follows a cetrain code style which you should also follow in your PR:
- indentation: one tab equals two spaces
- max line width is set to 80 characters
- If for instance a function declaration does not fit the line you should wrap it differently:
public static func == (
lhs: Result<Value, Error>,
rhs: Result<Value, Error>
) -> Bool {
|
@DevAndArtist I've updated to match the style you mentioned. |
|
I think I've created all of the API I need. Should I also do the inline documentation? |
stdlib/public/core/Result.swift
Outdated
There was a problem hiding this comment.
Maybe we should follow other files to change "2017" to "2018" here.
beccadax
left a comment
There was a problem hiding this comment.
The tests could use a little more work, but the actual implementation looks good.
stdlib/public/core/GroupInfo.json
Outdated
There was a problem hiding this comment.
Rather than making a category just for Result, why not make an "Errors" category, move ErrorType.swift and Result.swift into it, and move it above "Misc"?
There was a problem hiding this comment.
I'll do whatever the team wants here. I just added a separate Result category because Optional had its own category.
stdlib/public/core/Result.swift
Outdated
There was a problem hiding this comment.
Would it make sense to provide ~= (switch-matching) operators for Value: Equatable and Error patterns? (Hmm, technically that should be part of the proposal if it's included...)
Update: This PR has been closed in favor of the revised version #20958. See also: the second review thread.
This PR adds
Result<Value, Error>, as an implementation of Adding Result to the Standard Library.This PR has basic tests at the moment, suggestions for additional tests and proper annotations are welcome.