yomorun / hashids-java

Hashids algorithm v1.0.0 implementation in Java
http://hashids.org
MIT License
1.02k stars 156 forks source link

performance improvements #53

Open cazacugmihai opened 6 years ago

cazacugmihai commented 6 years ago

This change is Reviewable

0x3333 commented 6 years ago

Could you fix formatting, otherwise it's impossible to analyze the diff with so many changes. Thanks.

0x3333 commented 6 years ago

Also, if you could explain the performance improvements would help.

cazacugmihai commented 6 years ago

I have fixed the formatting.

The performance consists in:

cazacugmihai commented 6 years ago

About the performance: I have gained about 100-200 ms for running all the tests (it is at least 20% more efficient).

0x3333 commented 6 years ago

Great, I'll take some time to review and get back to you. Thanks.

cazacugmihai commented 6 years ago

About breaking this pull request in smaller pieces: I can't do this because the most part of it is related to the migration from String to char[]. I may extract the constants and the validation method but it will not help much reviewing the code (because those changes are only a couple of lines).

0x3333 commented 6 years ago

@cazacugmihai what do you think about creating a branch and work on a new version? @gabrielhora you are invited to this discussion. I'll create an issue to work on that.

cazacugmihai commented 6 years ago

Sure, I can do that.

0x3333 commented 6 years ago

Issue 55 - V2 Proposal. Let's discuss there before starting to code.