API redesign #9
Comments
This was referenced Jul 23, 2017
Closed
Closed
Closed
arcticicestudio
added a commit
that referenced
this issue
Jul 30, 2017
The POJO has been implemented to simplify the handling of the binding between the numbers and their hash value, but users should implement the simple POJO on their own instead of forcing them to use this approach. The public API methods now returning the primitive values which have been hold by the Hashid class. GH-9
arcticicestudio
added a commit
that referenced
this issue
Jul 30, 2017
* "decodeLongNumbers(String):long[]" The removed class "com.arcticicestudio.icecore.hashids.Hashid" has been used as a kind of container to store both the numbers and their associated hash values. This method allowed to directly receive the Long numbers instead of the Hashid POJO which will now be provided by the "decode(String):long[]" method. * "decodeIntegerNumbers(String):int[]" The algorithm The is designed for Long numbers. This method has been implemented to provide an easy conversion interface to use the library with integer IDs, but this should be handled by the user. * "encodeToString(long...):String" and "encodeToString(int...):String" This methods have are the counterpart for the decode logic methods described above. Both have been removed for the same reason and the functionality will now be available through the "encode(String):long[]" method. GH-9
arcticicestudio
added a commit
that referenced
this issue
Jul 30, 2017
The curse word protection of the original algorithm implementation does not allow to modify the separators. This API provided methods to customize those causing invalid and inconsistent hash- and numbers values. GH-9
arcticicestudio
added a commit
that referenced
this issue
Jul 30, 2017
The overloaded constructors are not necessary since the Hashids.Builder class provides methods for a accurate instance configuration. It allows a finer grained building since it doesn't depends on parameter ordering. The parameterless constructor is still available for brevity and the functionality of all other removed constructors can be achieved with the builder. GH-9
arcticicestudio
added a commit
that referenced
this issue
Jul 30, 2017
Due to the relations between the changes it is necessary to keep those in one commit which are explained in detail below. The main change of this revise is the clean rewrite of the internal API logic. Public API breaking changes: * All parameterized constructors have been removed due to the already provided interfaces through the Hashids.Builder class. It provides methods to achieve the same instance configuration, an instance with the default interoperable configurations is still available through the default constructor or by using default constructed builder instance. * The Hashids.Builder methods "minHashLength(int) : Builder" has been renamed to "minLength(int) : Builder" to adapt to the the builder methods naming style. * The methods * encodeToString(long...) : String * encodeToString(int...) : String * decodeLongNumbers(String) : long[] * decodeIntegerNumbers(String) : int[] are unnecessary now due to the removement of the "Hashid" class and have also been removed. The internal API has been redesigned and rewritten from scratch regarding optimizations for Java 8, reduction of complexity, performance and the new code style guide conventions. Internal API changes: * The private main logic methods "doEncode(long...)" and "doDecode(String, String)" have been removed and rewritten from scratch into the associated public API methods. * The private method "consistentShuffle(String, String) : String" has been rewritten from scratch and renamed to "shuffle(char[], char[]) : char[]". * The private methods "hash(long, String)" and "unhash(String, String)" have been rewritten from scratch and renamed to "transform(long, char[], StringBuilder, int)" and "transform(char[], char"])". * The primary algorithm logic, which was mainly implemented in the "doEncode" and "doDecode" private methods, has been rewritten from scratch and modularized into the new methods "deriveNewAlphabet(char[], char[], char) : char[]" and "filterSeparators(char[], char[]) : char[]". * The private support methods "toArray(List<Long>) : long[]" and "isEmpty(String) : boolean" have been removed. The new "HashidsFeature" enum contains constants for various features which can be enabled per instance through the Hashids.Builder method "features(HashidFeature) : Builder". Next to the changes listed above the public API now provides the "decodeOne(String) : Optional<Long>" method to simplify the use-case where the amount of resulting numbers is known before to handle the return value as single value instead of an array. This improves the prevention of an ArrayIndexOutOfBounceException when the user tries to access an non-existent array index due to an failed encoding- or decoding. All unit tests have also been rewritten from scratch to match the new API and include as much use-cases as possible and increase the code coverage. GH-9
arcticicestudio
added a commit
that referenced
this issue
Jul 30, 2017
Some generated hashes are invalid, but only differ by one or two characters to the valid hash. This has been caused by two logical problems during the stream which transforms each number into its hash representation: * The "lottery" character has been appended in the first loop causing a change in index numbers used by the followed code. Instead of the valid character at index 1 the lottery character has been used which caused invalid calculation results. * Instead of using the current loop round count (idx) to increment a calculated number it has been always incremented by 1 only causing all follow up calculations to use a wrong value. This commits fixes both bugs and ensures that the correct separator character is now added to the calculated hash fragments while the lottery character in prepended afterwards. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 1, 2017
Due to the relations between the changes it is necessary to keep those in one commit which are explained in detail below. The main change of this revise is the clean rewrite of the internal API logic. Public API breaking changes: * All parameterized constructors have been removed due to the already provided interfaces through the Hashids.Builder class. It provides methods to achieve the same instance configuration, an instance with the default interoperable configurations is still available through the default constructor or by using default constructed builder instance. * The Hashids.Builder methods "minHashLength(int) : Builder" has been renamed to "minLength(int) : Builder" to adapt to the the builder methods naming style. * The methods * encodeToString(long...) : String * encodeToString(int...) : String * decodeLongNumbers(String) : long[] * decodeIntegerNumbers(String) : int[] are unnecessary now due to the removement of the "Hashid" class and have also been removed. The internal API has been redesigned and rewritten from scratch regarding optimizations for Java 8, reduction of complexity, performance and the new code style guide conventions. Internal API changes: * The private main logic methods "doEncode(long...)" and "doDecode(String, String)" have been removed and rewritten from scratch into the associated public API methods. * The private method "consistentShuffle(String, String) : String" has been rewritten from scratch and renamed to "shuffle(char[], char[]) : char[]". * The private methods "hash(long, String)" and "unhash(String, String)" have been rewritten from scratch and renamed to "transform(long, char[], StringBuilder, int)" and "transform(char[], char"])". * The primary algorithm logic, which was mainly implemented in the "doEncode" and "doDecode" private methods, has been rewritten from scratch and modularized into the new methods "deriveNewAlphabet(char[], char[], char) : char[]" and "filterSeparators(char[], char[]) : char[]". * The private support methods "toArray(List<Long>) : long[]" and "isEmpty(String) : boolean" have been removed. The new "HashidsFeature" enum contains constants for various features which can be enabled per instance through the Hashids.Builder method "features(HashidFeature) : Builder". Next to the changes listed above the public API now provides the "decodeOne(String) : Optional<Long>" method to simplify the use-case where the amount of resulting numbers is known before to handle the return value as single value instead of an array. This improves the prevention of an ArrayIndexOutOfBounceException when the user tries to access an non-existent array index due to an failed encoding- or decoding. All unit tests have also been rewritten from scratch to match the new API and include as much use-cases as possible and increase the code coverage. The interoperability tests run against the latest version of the reference implementation "hashids.js". The script is loaded by using thge maven-frontend-plugin which installs NodeJS locally and runs NPM to install all dependencies defined in the package.json. The parameterized "InteropHashidsTest" class compares the results of both algorithms to ensure interoperability. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 1, 2017
Some generated hashes are invalid, but only differ by one or two characters to the valid hash. This has been caused by two logical problems during the stream which transforms each number into its hash representation: * The "lottery" character has been appended in the first loop causing a change in index numbers used by the followed code. Instead of the valid character at index 1 the lottery character has been used which caused invalid calculation results. * Instead of using the current loop round count (idx) to increment a calculated number it has been always incremented by 1 only causing all follow up calculations to use a wrong value. This commits fixes both bugs and ensures that the correct separator character is now added to the calculated hash fragments while the lottery character in prepended afterwards. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 2, 2017
Due to the relations between the changes it is necessary to keep those in one commit which are explained in detail below. The main change of this revise is the clean rewrite of the internal API logic. Public API breaking changes: * All parameterized constructors have been removed due to the already provided interfaces through the Hashids.Builder class. It provides methods to achieve the same instance configuration, an instance with the default interoperable configurations is still available through the default constructor or by using default constructed builder instance. * The Hashids.Builder methods "minHashLength(int) : Builder" has been renamed to "minLength(int) : Builder" to adapt to the the builder methods naming style. * The methods * encodeToString(long...) : String * encodeToString(int...) : String * decodeLongNumbers(String) : long[] * decodeIntegerNumbers(String) : int[] are unnecessary now due to the removement of the "Hashid" class and have also been removed. The internal API has been redesigned and rewritten from scratch regarding optimizations for Java 8, reduction of complexity, performance and the new code style guide conventions. Internal API changes: * The private main logic methods "doEncode(long...)" and "doDecode(String, String)" have been removed and rewritten from scratch into the associated public API methods. * The private method "consistentShuffle(String, String) : String" has been rewritten from scratch and renamed to "shuffle(char[], char[]) : char[]". * The private methods "hash(long, String)" and "unhash(String, String)" have been rewritten from scratch and renamed to "transform(long, char[], StringBuilder, int)" and "transform(char[], char"])". * The primary algorithm logic, which was mainly implemented in the "doEncode" and "doDecode" private methods, has been rewritten from scratch and modularized into the new methods "deriveNewAlphabet(char[], char[], char) : char[]" and "filterSeparators(char[], char[]) : char[]". * The private support methods "toArray(List<Long>) : long[]" and "isEmpty(String) : boolean" have been removed. The new "HashidsFeature" enum contains constants for various features which can be enabled per instance through the Hashids.Builder method "features(HashidFeature) : Builder". Next to the changes listed above the public API now provides the "decodeOne(String) : Optional<Long>" method to simplify the use-case where the amount of resulting numbers is known before to handle the return value as single value instead of an array. This improves the prevention of an ArrayIndexOutOfBounceException when the user tries to access an non-existent array index due to an failed encoding- or decoding. All unit tests have also been rewritten from scratch to match the new API and include as much use-cases as possible and increase the code coverage. The interoperability tests run against the latest version of the reference implementation "hashids.js". The script is loaded by using thge maven-frontend-plugin which installs NodeJS locally and runs NPM to install all dependencies defined in the package.json. The parameterized "InteropHashidsTest" class compares the results of both algorithms to ensure interoperability. All CI build configurations have been updated to match the new tests. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 2, 2017
Some generated hashes are invalid, but only differ by one or two characters to the valid hash. This has been caused by two logical problems during the stream which transforms each number into its hash representation: * The "lottery" character has been appended in the first loop causing a change in index numbers used by the followed code. Instead of the valid character at index 1 the lottery character has been used which caused invalid calculation results. * Instead of using the current loop round count (idx) to increment a calculated number it has been always incremented by 1 only causing all follow up calculations to use a wrong value. This commits fixes both bugs and ensures that the correct separator character is now added to the calculated hash fragments while the lottery character in prepended afterwards. GH-9
Merged
arcticicestudio
added a commit
that referenced
this issue
Aug 5, 2017
Due to the relations between the changes it is necessary to keep those in one commit which are explained in detail below. The main change of this revise is the clean rewrite of the internal API logic. Public API breaking changes: * All parameterized constructors have been removed due to the already provided interfaces through the Hashids.Builder class. It provides methods to achieve the same instance configuration, an instance with the default interoperable configurations is still available through the default constructor or by using default constructed builder instance. * The Hashids.Builder methods "minHashLength(int) : Builder" has been renamed to "minLength(int) : Builder" to adapt to the the builder methods naming style. * The methods * encodeToString(long...) : String * encodeToString(int...) : String * decodeLongNumbers(String) : long[] * decodeIntegerNumbers(String) : int[] are unnecessary now due to the removement of the "Hashid" class and have also been removed. The internal API has been redesigned and rewritten from scratch regarding optimizations for Java 8, reduction of complexity, performance and the new code style guide conventions. Internal API changes: * The private main logic methods "doEncode(long...)" and "doDecode(String, String)" have been removed and rewritten from scratch into the associated public API methods. * The private method "consistentShuffle(String, String) : String" has been rewritten from scratch and renamed to "shuffle(char[], char[]) : char[]". * The private methods "hash(long, String)" and "unhash(String, String)" have been rewritten from scratch and renamed to "transform(long, char[], StringBuilder, int)" and "transform(char[], char"])". * The primary algorithm logic, which was mainly implemented in the "doEncode" and "doDecode" private methods, has been rewritten from scratch and modularized into the new methods "deriveNewAlphabet(char[], char[], char) : char[]" and "filterSeparators(char[], char[]) : char[]". * The private support methods "toArray(List<Long>) : long[]" and "isEmpty(String) : boolean" have been removed. The new "HashidsFeature" enum contains constants for various features which can be enabled per instance through the Hashids.Builder method "features(HashidFeature) : Builder". Next to the changes listed above the public API now provides the "decodeOne(String) : Optional<Long>" method to simplify the use-case where the amount of resulting numbers is known before to handle the return value as single value instead of an array. This improves the prevention of an ArrayIndexOutOfBounceException when the user tries to access an non-existent array index due to an failed encoding- or decoding. All unit tests have also been rewritten from scratch to match the new API and include as much use-cases as possible and increase the code coverage. The interoperability tests run against the latest version of the reference implementation "hashids.js". The script is loaded by using thge maven-frontend-plugin which installs NodeJS locally and runs NPM to install all dependencies defined in the package.json. The parameterized "InteropHashidsTest" class compares the results of both algorithms to ensure interoperability. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 5, 2017
Some generated hashes are invalid, but only differ by one or two characters to the valid hash. This has been caused by two logical problems during the stream which transforms each number into its hash representation: * The "lottery" character has been appended in the first loop causing a change in index numbers used by the followed code. Instead of the valid character at index 1 the lottery character has been used which caused invalid calculation results. * Instead of using the current loop round count (idx) to increment a calculated number it has been always incremented by 1 only causing all follow up calculations to use a wrong value. This commits fixes both bugs and ensures that the correct separator character is now added to the calculated hash fragments while the lottery character in prepended afterwards. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 5, 2017
Due to the relations between the changes it is necessary to keep those in one commit which are explained in detail below. The main change of this revise is the clean rewrite of the internal API logic. Public API breaking changes: * All parameterized constructors have been removed due to the already provided interfaces through the Hashids.Builder class. It provides methods to achieve the same instance configuration, an instance with the default interoperable configurations is still available through the default constructor or by using default constructed builder instance. * The Hashids.Builder methods "minHashLength(int) : Builder" has been renamed to "minLength(int) : Builder" to adapt to the the builder methods naming style. * The methods * encodeToString(long...) : String * encodeToString(int...) : String * decodeLongNumbers(String) : long[] * decodeIntegerNumbers(String) : int[] are unnecessary now due to the removement of the "Hashid" class and have also been removed. The internal API has been redesigned and rewritten from scratch regarding optimizations for Java 8, reduction of complexity, performance and the new code style guide conventions. Internal API changes: * The private main logic methods "doEncode(long...)" and "doDecode(String, String)" have been removed and rewritten from scratch into the associated public API methods. * The private method "consistentShuffle(String, String) : String" has been rewritten from scratch and renamed to "shuffle(char[], char[]) : char[]". * The private methods "hash(long, String)" and "unhash(String, String)" have been rewritten from scratch and renamed to "transform(long, char[], StringBuilder, int)" and "transform(char[], char"])". * The primary algorithm logic, which was mainly implemented in the "doEncode" and "doDecode" private methods, has been rewritten from scratch and modularized into the new methods "deriveNewAlphabet(char[], char[], char) : char[]" and "filterSeparators(char[], char[]) : char[]". * The private support methods "toArray(List<Long>) : long[]" and "isEmpty(String) : boolean" have been removed. The new "HashidsFeature" enum contains constants for various features which can be enabled per instance through the Hashids.Builder method "features(HashidFeature) : Builder". Next to the changes listed above the public API now provides the "decodeOne(String) : Optional<Long>" method to simplify the use-case where the amount of resulting numbers is known before to handle the return value as single value instead of an array. This improves the prevention of an ArrayIndexOutOfBounceException when the user tries to access an non-existent array index due to an failed encoding- or decoding. All unit tests have also been rewritten from scratch to match the new API and include as much use-cases as possible and increase the code coverage. The interoperability tests run against the latest version of the reference implementation "hashids.js". The script is loaded by using thge maven-frontend-plugin which installs NodeJS locally and runs NPM to install all dependencies defined in the package.json. The parameterized "InteropHashidsTest" class compares the results of both algorithms to ensure interoperability. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 5, 2017
Some generated hashes are invalid, but only differ by one or two characters to the valid hash. This has been caused by two logical problems during the stream which transforms each number into its hash representation: * The "lottery" character has been appended in the first loop causing a change in index numbers used by the followed code. Instead of the valid character at index 1 the lottery character has been used which caused invalid calculation results. * Instead of using the current loop round count (idx) to increment a calculated number it has been always incremented by 1 only causing all follow up calculations to use a wrong value. This commits fixes both bugs and ensures that the correct separator character is now added to the calculated hash fragments while the lottery character in prepended afterwards. GH-9
arcticicestudio
added a commit
that referenced
this issue
Aug 5, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Public API
The public API is currently designed using a OOP layout by returning the
Hashidclass which stores the numbers and the hash value. It may be useful in some situations where the user like to store objects instead of primitives, but sue to the simplicity of theHashidclass it is easy for any user to implement such a small class on their own.➡️ Redesign the API to only process (boxed) primitive types for both parameters and return types which will replace the
Hashidclass.Internal API
The internal API is unnecessary complicated when it comes to method complexity. The code style is too similar to the original implementation regarding variable naming and code structures. It does not make use of the advantages of Java 8 like streams and lambda expressions which are a great improvement for the performance of the algorithm.
➡️ The API should be redesigned regarding optimizations for Java 8 and the reduction of complexity for both public- and private methods.
Tests
The current code coverage is too low and should be increased by implementing unit tests for uncovered- and partially covered code blocks. The unit tests should also make use of JUnit's
@Parameterizedannotation to simplify each test case.➡️ Add a test suite which runs the latest original hashids.js implementation against the results of this library to ensure full compatibility and integrity. This should be done by using the frontend-maven-plugin to make use of Node's npm ecosystem.
The text was updated successfully, but these errors were encountered: