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

reimplemented unhash using Horner's method #34

Closed
wants to merge 1 commit into from

Conversation

jkramarz
Copy link

@jkramarz jkramarz commented Jan 1, 2017

This change is Reviewable

@0x3333
Copy link
Collaborator

0x3333 commented Jan 2, 2017

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


src/main/java/org/hashids/Hashids.java, line 386 at r1 (raw file):

    long number = 0;

    for (char item: input.toCharArray()) {

Could you implement without calling toCharArray()? Using charAt is much more efficient. Despite that, code is good, nice catch.


src/main/java/org/hashids/Hashids.java, line 387 at r1 (raw file):

    for (char item: input.toCharArray()) {
      long position = alphabet.indexOf(item);

Also, you could use the same pos variable instead of declaring inside the loop. This would make minimal modifications to code.


Comments from Reviewable

@0x3333
Copy link
Collaborator

0x3333 commented Feb 7, 2017

Hi @jkramarz any news on the modifications? Would be a nice addition to the project.

Thanks!

@0x3333
Copy link
Collaborator

0x3333 commented May 29, 2017

I'll close as we changed directly in master. Thanks @jkramarz.

@0x3333 0x3333 closed this May 29, 2017
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.

2 participants