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

refactor int conversion #45

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DoTheBestToGetTheBest
Copy link

  • this PR refactors the to_safe_integer and to_u32 functions into a generic to_integer function

Copy link
Contributor

@kaisbaccour kaisbaccour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? Looks more complicated than before. Introducing TryFrom is not needed in the original code

@DoTheBestToGetTheBest
Copy link
Author

Why this change? Looks more complicated than before. Introducing TryFrom is not needed in the original code

hello, yes i used tryfrom for fallible conversions

when using tryfrom it attempts to convert one type into another, but unlike a direct cast, it can handle cases where the conversion might not be possible

if you are not okey with TryFrom i could remove it :)

thanks

@webmaster128
Copy link
Contributor

I'm primarily wondering what the advantage of the change is

@DoTheBestToGetTheBest
Copy link
Author

DoTheBestToGetTheBest commented Jan 8, 2024

I'm primarily wondering what the advantage of the change is

hey, so basically, it handles errors that can occur during type conversion

ref : https://doc.rust-lang.org/std/convert/trait.TryFrom.html

thanks:)

@webmaster128
Copy link
Contributor

Right, but we already checked

  1. the input is an integer
  2. the input is in the expected value range

So the cast to u32 is always safe. What other errors can happen here that T::try_from(converted).ok() handles?

@DoTheBestToGetTheBest
Copy link
Author

Right, but we already checked

  1. the input is an integer
  2. the input is in the expected value range

So the cast to u32 is always safe. What other errors can happen here that T::try_from(converted).ok() handles?

for example when converting a f64 to a smaller integer type , there's a risk of overflow. TryFrom safely handles these cases by returning an Err

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 this pull request may close these issues.

3 participants