Closed cmaloney closed 7 years ago
@speps any chance you could look at this soon?
I'll try to have a look tonight, sorry about the delay, I'm quite busy at the moment :)
Sounds good, thanks!
I had a quick review, looks really nice! Thanks!
Thanks for reviewing and merging! Happy to get the org I work off back off my temporary branch
hashids is in the middle of a fairly performance sensitive bit of code where I work (Returning thousands of search results, each result id gets encoded). The allocations which happen end up taking a significant amount of the time, so this PR works to reduce the number of allocations per execution from 31 + varying based on the input data to 5 always, while trying to keep the code reasonable to read / work through. The company's code using this doesn't use
MinLength
, so that I haven't spent a lot of time optimizing, and has more allocations which could probably be removed (Although less than it used to). There is a test added which should make it easier for someone to improve if they want down the line, as well as make sure it doesn't regress.Overall: