Performance optimizations and style

philnash / pwned

😱 An easy, Ruby way to use the Pwned Passwords API.

https://rubygems.org/gems/pwned/

MIT License

424 stars 22 forks source link

Performance optimizations and style #2

Closed kpumuk closed 6 years ago

kpumuk commented 6 years ago

This PR includes:

SHA1 is only calculated once
Frozen string literal to make sure Ruby does not copy strings over and over again
Removal of @match_data, since we only use it to retrieve the counter. Caching the counter instead (this way we do not re-apply regex over and over again if there is no match [should not happen, but still])
~Unnecessary begin/end in get_hashes~

philnash commented 6 years ago

Thanks! There's some good stuff here.

Going to merge this, but I really prefer the begin/rescue/end way of error handling. It's mainly just the way it looks, I like the intention of saying "here is where I am going to rescue errors" rather than surprising the reader after a potential source of error. I'm going to keep that like it is. Otherwise you definitely caught a good bunch of issues here. Thanks!

kpumuk commented 6 years ago

Ok, let me fix that

philnash commented 6 years ago

Well, I rebased and messed up this PR. Any ideas on how to fix it? I want to attribute the PR to you, not just close this. Perhaps you can pull the latest in master and force push to this branch, which should trigger GitHub to recognise the merge. (And I won't rebase again.)

kpumuk commented 6 years ago

Sure.

git reset --hard 062843efcb22863727074f9dbcb9cde60864009c
git push -f

This will essentially discard all the commits you have made after the initial "Adding a changelog" (which is rebase + changelog update, which I pulled into this PR just now)

kpumuk commented 6 years ago

Another option is to close this PR, and I will work to resolve the conflicts with my other branches (this PR is not the last one :-) ). Please let me know which way do you prefer.