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

Functions should return Result not Option #85

Open
ijackson opened this issue Aug 7, 2023 · 1 comment
Open

Functions should return Result not Option #85

ijackson opened this issue Aug 7, 2023 · 1 comment

Comments

@ijackson
Copy link

ijackson commented Aug 7, 2023

We had a user report a bad error message from some Arti libraries on Android. The root cause of the problem was #83, but additionally, I felt that the error message from our library was too poor. (https://gitlab.torproject.org/tpo/core/arti/-/issues/ 989 999)

I investigated and found that the main reason the message was poor was because ProjectDirs::from simply returned None. So we had to make up our own, rather useless, error message.

IMO all the fallible functions should return Result. (This would be a semver-breaking change, obviously.)

@sidju
Copy link

sidju commented Nov 12, 2023

In an attempt to rephrase this slightly more clearly (since I was confused and opposed until I understood):

Functions would better return Result<Option>, so that:

  • Err can represent when some part of the process to construct the path or identify if it is valid for the platform failed
  • Ok(None) can represent when the library successfully concluded there is no such path for the platform
  • Ok(Some(path)) can represent when a valid path for the platform was successfully constructed

This would allow easy error propagation with ? or .expect() while otherwise keeping user code intact, and would make failure causes more clear.

(Edit: Removed semi-related suggestion that was already implemented in the API.)

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

No branches or pull requests

2 participants