Skip to content

Conversation

arcticicestudio
Copy link
Owner

Closes #9

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
* "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
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
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 arcticicestudio added this to the 0.4.0 milestone Aug 2, 2017
@arcticicestudio arcticicestudio self-assigned this Aug 2, 2017
@codecov
Copy link

codecov bot commented Aug 2, 2017

Codecov Report

Merging #22 into develop will increase coverage by 12.21%.
The diff coverage is 92.5%.

Impacted file tree graph

@@              Coverage Diff               @@
##             develop      #22       +/-   ##
==============================================
+ Coverage      80.54%   92.75%   +12.21%     
- Complexity        56       89       +33     
==============================================
  Files              2        2               
  Lines            221      276       +55     
  Branches          38       52       +14     
==============================================
+ Hits             178      256       +78     
+ Misses            22        6       -16     
+ Partials          21       14        -7
Impacted Files Coverage Δ Complexity Δ
...rcticicestudio/icecore/hashids/HashidsFeature.java 100% <100%> (ø) 1 <1> (?)
...a/com/arcticicestudio/icecore/hashids/Hashids.java 92.61% <92.36%> (+13.61%) 88 <88> (+41) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fa3d22...fb1caa9. Read the comment docs.

@arcticicestudio arcticicestudio force-pushed the feature/gh-9-api-redesign branch from 0eede86 to 71472f9 Compare August 5, 2017 07:43
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
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 arcticicestudio force-pushed the feature/gh-9-api-redesign branch from 71472f9 to fb1caa9 Compare August 5, 2017 07:53
@arcticicestudio arcticicestudio merged commit bf6971c into develop Aug 5, 2017
@arcticicestudio arcticicestudio deleted the feature/gh-9-api-redesign branch August 5, 2017 08:03
@arcticicestudio arcticicestudio removed their assignment Sep 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant