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

Why do some functions panic instead of returning a Result<T, E>? #556

Closed
BlueSialia opened this issue Apr 20, 2021 · 10 comments
Closed

Why do some functions panic instead of returning a Result<T, E>? #556

BlueSialia opened this issue Apr 20, 2021 · 10 comments

Comments

@BlueSialia
Copy link

Right now I'm parsing some information I receive in UDP messages. Some of that information is timestamped but if something goes wrong on the sender I can receive random bytes so if I try to parse those bytes as dates I get a panic with 'No such local time'.

And I wonder. If the function Local.ymd(y, month, d).and_hms(h, min, s) can fail why not return a Result instead?

@BlueSialia BlueSialia changed the title Why do some functions panic insteaf of returning a Result<T, E>? Why do some functions panic instead of returning a Result<T, E>? Apr 20, 2021
@dzfranklin
Copy link

I think those functions are intended for building timestamps from known good values. You might be looking for DateTime::parse_from_str which lets you specify the format you're trying to parse in the standard unix-y way and returns a result. Don't take me too seriously, I'm a random person who saw this while filing their own question.

@BlueSialia
Copy link
Author

I see where you are coming from. But I do not have a String, though. I could create it to then parse it, but that feels like an ugly hack.

Regardless, I was under the assumption that Rust code practices were about working with Result and Option whenever some function could fail. That Rust is reliable because runtime errors should only happen if the programmer uses unwrap() (an similar functions) or unsafe code.

@deviant
Copy link

deviant commented Jul 23, 2021

The time crate (which this depends on) placed panicking functions behind a feature gate and switched to providing try_ functions instead. This should too.

@c-git
Copy link
Contributor

c-git commented Jan 28, 2023

Is there something I can do to move this forward? I prefer not need to check each function to see if it might panic. Maybe offer parallel functions that do not panic so it's not a breaking change?

@djc
Copy link
Member

djc commented Jan 28, 2023

Feel free to submit PRs to create non-panicking alternative APIs en add deprecations for the panicking calls.

@c-git
Copy link
Contributor

c-git commented Jan 29, 2023

Ok thank you, I will try to do it. Just to get a feel for naming preference, should I just add try_ to the front of the functions I change?

@djc
Copy link
Member

djc commented Jan 29, 2023

That seems like a reasonable first approximation. Our constructors mostly have _opt() suffixes at this point, let's see what works in context.

@c-git
Copy link
Contributor

c-git commented Jan 29, 2023

Got it, will try to see if I can find a pattern. The name can always be refactored, I don't get attached to them

@c-git
Copy link
Contributor

c-git commented Feb 17, 2023

Haven't forgotten about this but my schedule has been kind of full so if someone else gets to it first that's fine otherwise I will give it a go once I can put some time aside.

@pitdicker
Copy link
Collaborator

Closing in favor of #1049. We have non-panicking alternatives to almost all methods now, and making them default for 0.5 is definitely on the roadmap.

@pitdicker pitdicker closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2024
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

6 participants