Skip to content

Conversation

@randolf
Copy link

@randolf randolf commented Jan 16, 2019

Added a new Hashids constructor to support customized "separators" plus a pre-check for NULL that prevents a NullException by defaulting to DEFAULT_SEPS (this behaviour is modeled after the "salt" pre-check for NULL).

Added a pre-check for NULL for the custom "alphabet" that prevents a NullException by defaulting to DEFAULT_ALPHABET (this behaviour is also modeled after the "salt" pre-check for NULL).

Added rudimentary comments for existing constructors for better Javadoc output. Added relevant details about the new constructor that supports custom "separators" (should this also be mentioned in the documentation on the web site?).

…t blocks to all constructors

Added a new Hashids constructor to support customized "separators" plus a pre-check for NULL that prevents a NullException by defaulting to DEFAULT_SEPS (this behaviour is modeled after the "salt" pre-check for NULL).

Added a pre-check for NULL for the custom "alphabet" that prevents a NullException by defaulting to DEFAULT_ALPHABET (this behaviour is also modeled after the "salt" pre-check for NULL).

Added rudimentary comments for existing constructors for better Javadoc output.  Added relevant details about the new constructor that supports custom "separators" (should this also be mentioned in the documentation on the web site?).
* @param alphabet (NULL defaults to "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")
*/
public Hashids(String salt, int minHashLength, String alphabet) {
this(salt, minHashLength, DEFAULT_ALPHABET, DEFAULT_SEPS);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alphabet isn't used here.

this.salt = salt != null ? salt : DEFAULT_SALT;
this.minHashLength = minHashLength > 0 ? minHashLength : DEFAULT_MIN_HASH_LENGTH;
if (alphabet == null) alphabet = DEFAULT_ALPHABET;
if (seps == null) seps = DEFAULT_SEPS;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should follow the pattern above that was used for salt i.e.

this.alphabet = alphabet != null ? alphabet : DEFAULT_ALPHABET;

@0x3333
Copy link
Collaborator

0x3333 commented May 20, 2021

@randolf , are you still able to fix those issues raised by @Jon-Hopper?

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.

3 participants