Conversation
Unit Test Results 9 files - 3 1 869 suites - 619 36m 13s ⏱️ - 12m 37s Results for commit c25677f. ± Comparison against base commit 2e451a4. ♻️ This comment has been updated with latest results. |
| peer = peer, status = remoteCgcResult.status | ||
| peer.updateScore(PeerScoreNoValues) | ||
| return intersection | ||
| let remoteCustodyGroupCount = remoteCgcResult.value |
There was a problem hiding this comment.
This pattern is partly why Result exists though.
That is, yes, on inspection this is correct, but it's still not encapsulating the in-principle-unsafe case object access. Result has the ?, valueOr, etc helpers which, yes, ultimately do something similar, but are tested separately to be correct in this capacity. This code contains the equivalent of a direct foo.get or foo.unsafeGet, even if it is guarded appropriately.
The purpose of Result, then, is partly do allow ultimately something like:
let remoteCustodyGroupCount = remoteCgcResult.valueOr:
debug "Failed to lookup cgc from peer",
peer = peer, status = remoteCgcResult.status
peer.updateScore(PeerScoreNoValues)
return intersectionto be written instead.
Ideally, the Nimbus codebase can, consistently with broad efficiency, minimize direct/statically-unsafe (treating nim-results as safe, for this purpose, because it is at least tested), avoid this kind of code as much as feasible.
| return false | ||
| let | ||
| remoteCustodyGroupCount = peer.lookupCgcFromPeer() | ||
| remoteCustodyGroupCount = remoteCgcResult.value |
There was a problem hiding this comment.
Similarly to the other comment, this is another equivalent-to-.get/.unsafeGet code style which could cause a runtime Defect and would be improved by using Result or Opt (i.e. Result with an error type of void).
2 participants
No description provided.