Add definitions for == and != #21
Conversation
For Result types, equality should be defined based on the equality of the contained types. This adds the logic for == (flipped for !=) to allow you to compare two instances of Result.
|
Do you have a live example of where this has come up? I think the question So far result equality has never come in my code, so I'd like to see some |
Like you, I needed it in my tests. I haven't needed it in production yet, but I could see it potentially being useful in the future. I also think it's hard to say that it won't ever be useful. The implementation is light weight enough that it doesn't seem like it's worth avoiding.
I don't think that's LlamaKit's responsibility at all.
You would need to make your error type |
|
Once we set a semantic, it's very hard to change it later. I'd rather wait for a live need for the operator rather than forcing it upon people in a way that may be undesirable. I can imagine callers wanting all errors to be equal just as much as I can imagine they want them not to be equal. I can easily see bugs that would come about by callers not considering the error case in their equality checks or being surprised about the equality semantic. Callers can always define their own if they want to. The fact that no one so far has come up with a non-test need for it is, I believe, a strong argument to leave it out for now. LlamaKit very intentionally has a minimal footprint. My criteria is "it's necessary for good code" rather than "it's easy to implement." If it's useful, I expect it to pop up in a framework as large as ReactiveCocoa or in Brad Larson's work. Even after writing significant production code with Result, his didn't wind up having an ==. https://github.com/SonoPlot/SerialPortExample-Swift/blob/master/SerialPortExample-Swift/Result.swift @jspahrsummers @BradLarson Any input from your use? It's never come up for me. Is its lack likely to cause confusion (people implementing it their own incompatible ways?) I find by the time I care about values, I'm always in a switch statement. |
|
@rnapier - The only times that I can see needing an equality check for an entire Result type is in tests. In production code, you're going to be either operating against the encapsulated .Success value or handling the error case. You're going to know what half you want at a specific point in code. In unit tests, you'll want different messages for the three possible cases of Success / Failure mismatch, right Success / Failure case but wrong encapsulated value, and complete match. A simple pass / fail on Result equality isn't that helpful in diagnosing test failures. To that end, I created the assertResultsAreEqual() function for use in my XCTestCases: I've overloaded it to handle the various cases where the boxed success and error types themselves do or do not comply to the Equatable protocol (as well as a few specific cases for testing equality of byte arrays and Voids contained as values). This serves all my unit testing needs perfectly, and provides far more usable information in a test failure than a simple equality could. With one line, I can test and report on the various match / mismatches between two Results. Again, beyond testing, I see little need in my production code for an equality operator on Results themselves. |
|
IMO any type that has |
For
Resulttypes, equality should be defined based on the equality of the contained types. This adds the logic for==(flipped for!=) to allow you to compare two instances ofResult.I saw a mention from @rnapier on the mailing list about removing this operator implementation, but I don't fully understand why. I think this is a totally reasonable thing to have, and I was actually a little surprised when it didn't exist.