search

asmcrypto / asmcrypto.js

JavaScript Cryptographic Library with performance in mind.
MIT License
661 stars 182 forks source link

Factor out the common part of the PBKDF2 algorithm #160

Closed wiml closed 5 years ago

wiml commented 6 years ago

The three PBKDF2 functions were just duplicates of each other with one line changed. This PR combines them into one function with an extra argument.

Ruffio commented 6 years ago

@alippai how does this PR look? Ready to merged?

alippai commented 5 years ago

This didn't pass the TypeScript type checking, so reverted it back.

wiml commented 5 years ago

@alippai What was the error? I'm pretty sure I typechecked it before opening the PR.

The committed version seems a little bit brittle, since it has to list every PRF, instead of simply allowing any object of the correct type.

alippai commented 5 years ago

@wiml here is the failing commit, after turning on the error reporting: https://travis-ci.org/asmcrypto/asmcrypto.js/jobs/450175452

The new solution can be tuned, indeed. The overall quality of OOP is bad in this project, because it's based on legacy code.

wiml commented 5 years ago

Ah, I see! Well if minor tuning / cleanup PRs are accepted, I might make some when I have some spare time.