spenserblack / github-stats-rs

A tool for using Github's API
Apache License 2.0
1 stars 5 forks source link

prevent over/underflow in prev_page and next_page #12

Closed evlnyng closed 5 years ago

evlnyng commented 5 years ago

For issue #4. Thank you for your patience, I'm a newbie and still learning (slowly)! 😅

spenserblack commented 5 years ago

No problem! Although it looks like you've overthought the TODOs. I should've been more specific. You only need to make sure the page number is between the boundaries of std::usize::MIN and std::usize::MAX. 😆 It was a good idea to check if the next_page would go beyond total_count / per_page, though.

Also, a word of warning when using unwrap. This will cause the program to panic if search returns a Result::Err instead of Result::Ok. It's not so bad to panic in a binary (e.g. main.rs), but in a library that others may use, you want to avoid causing any panics. It should be up to the binary itself to decide whether to panic or to do something else. So you should really only use unwrap when you are really sure that the value you're calling it on will never be an Err. We do not know that search won't be an Err. An Err could be returned due to a connection timeout, for example.

Please make these changes. Looking forward to merging your PR!

evlnyng commented 5 years ago

I was a little worried that I may be doing that! 😅 Thank you so much for the explanation - I'm learning lots and I really appreciate it!