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

Parse into Option<T> #4

Closed
jhpratt opened this issue Feb 11, 2019 · 7 comments
Closed

Parse into Option<T> #4

jhpratt opened this issue Feb 11, 2019 · 7 comments
Labels

Comments

@jhpratt
Copy link

jhpratt commented Feb 11, 2019

This could be useful for things where an element may be present and have useful info, but is sometimes expected to not be present. My specific use case is ranking of sports teams — I currently have to catch the error and reparse into a different struct.

@Hexilee
Copy link
Owner

Hexilee commented Feb 11, 2019

It may be useful but currently I do not have the use case and time. Maybe you can set a "flag" as default value?

@jhpratt
Copy link
Author

jhpratt commented Feb 11, 2019

@Hexilee That's certainly a possibility. Would you accept a PR if I created one? I'd certainly take a look at the code if you would!

@Hexilee
Copy link
Owner

Hexilee commented Feb 12, 2019

PR is welcome! I will take some time to review the code and make the decision.

@jhpratt
Copy link
Author

jhpratt commented Feb 14, 2019

@Hexilee Looking through things now; it appears as though the only two errors that can be returned are SourceNotFound and SourceEmpty. If this is correct, would you agree that the appropriate thing to do is to cast both of them to an Option?

If that's the correct course of action, I actually think this should be as simple as calling .ok() on the result, as we're disregarding both possible errors.

@Hexilee
Copy link
Owner

Hexilee commented Feb 14, 2019

The purpose to use Result instead of Option is debugging more conveniently. Maybe you can use Option and log the err.

@Hexilee
Copy link
Owner

Hexilee commented Aug 21, 2019

@jhpratt Option<T> is supported in v0.7, documentation will be updated in few days, you can refer to the test compound.rs now.

@Hexilee
Copy link
Owner

Hexilee commented Aug 21, 2019

I will close this issue

@Hexilee Hexilee closed this as completed Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants