Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Result<(), ??> over ReturnCode #3

Closed
daboross opened this issue Mar 24, 2018 · 3 comments · Fixed by #417
Closed

Use Result<(), ??> over ReturnCode #3

daboross opened this issue Mar 24, 2018 · 3 comments · Fixed by #417

Comments

@daboross
Copy link
Collaborator

Making an issue here as a reminder.

It seems generally be more "rusty" to do this. The one disadvantage is having a two-byte return type size rather than one-byte, but that shouldn't be too large of a problem at all. We could even leave ReturnCode as a type alias for Result<(), ??>.

One question I'm not sure about is what to call the error portion. I'm thinking ErrorCode would work well, but just Error or GameError might also be good. I'm partially against Screeps* names because of duplication with screeps:: crate name.

daboross added a commit that referenced this issue Apr 5, 2018
This is actually a bit more and a bit less, since it no longe rlinks
to #3#. I've made the decision to keep a small part of my repository
closed source, so linking to it as a runnable example is probably not
the best idea!

If anyone else has fully open source examples, I'll be glad to add
them.
@daboross
Copy link
Collaborator Author

Seems there are some coherence issues implementing TryFrom<Value> for Result<(), ErrorCode>. Not a show stopper, but means we'll need to do a bit more to actually implement this.

In the mean time I'm going to add a utility method to go from ReturnCode to a Result<(), ReturnCode> after it's been gotten.

@ASalvail
Copy link
Collaborator

With return code built as enums, this becomes obsolete. Instead of having a two element enum (Ok or Err) we have a ~20 elements enum.

I'd move to close this. @daboross

@daboross
Copy link
Collaborator Author

daboross commented Jul 17, 2019

@ASalvail IIRC this has always been the case, there was never a time that ReturnCode wasn't a 20-element enum

The main advantages of Result that I was thinking of here are more from it being Result than from it being an enum in general. Like:

  • being able to use the ? operator to propogate
  • having combinator functions, like first_action().and_then(|()| second_action())
  • integrating with other error types - you could have impl From<ErrorCode> for YourError and then use move_to()? in a function returning Result<_, YourError>, correctly propagating the error code
  • other utility functions like Result::expect(message), or the implementation which allows Iterator::collect::<Result<Vec<_>, ErrType>>
  • making it even clearer that all except Ok is an error. I'm mostly thinking of the fact that returning a Result is used almost universally among rust crates, and the pattern of combining the error variants with the success variants in one enum is probably unique to this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants