Should we throw an exception for negative numbers as well?

[gabrielhora]

When encoding a number the library throw an exception if the number is loo long (greater than MAX_NUMBER) but return an empty string for negative numbers. IMHO this is a little unexpected, using two different behaviours for the same thing, input validation.

Since negative numbers are also illegal arguments, shouldn't it throw a IllegalArgumentException?

[cazacugmihai]

I'm of the same opinion (it can be seen here).

[0x3333]

I agree in the different behaviours, but I believe we should return an empty string. We must adhere to the original JS implementation, and in this case, it doesn't throw an exception. The original implementation doesn't have this limitation, but to have the same output in all implementations, we must limit the max to the JS max integer(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER).

[gabrielhora]

Hi @0x3333, thanks for taking the time to check the issue.

Is there a specific reason why the behaviour should be the same as the JS lib? Maybe this is more common in JS but returning an empty string is a very weird way to define a erroneous input in Java, I think anyone working with this library in Java would expect an exception, or maybe a null (or Optional for Java 8+).

[cazacugmihai]

In Java, for an inappropriate parameter, is thrown an exception (IllegalArgumentException). We should follow this rule.

[0x3333]

@gabrielhora the reason is that all libraries implementing hashids do this. To be truth, this is not something I like, but I stick to it, something like rule of thumb.

For example, in all implementations(Here some examples, C, Swift, Python), the unique alphabet is not throw an exception, error, or similar, they are just ignored. Or when there is no numbers C,Swift,Python. If you go deeper, even some error messages are the same.

This is the de facto regarding hashids, not a rule I created.

What we could do is create a new version, like 2.0, without those constraints, more java-ish, but current implementation I'd like to stick to it, as it is deployed for several years like that, and changing this behavior now is not acceptable.

@cazacugmihai I agree, but as I said before, this is the rule of thumb.

Guys, I'm really would like to help on this, but we should work on a version 2.0 to address these issues, current version cannot change this behavior.

[cazacugmihai]

As long as you'll fix these issues in the next version (2.0), I think that it's okay.

[arcticicestudio]

I also bothered with this limitations so I wrote a own implementation (listed on http://hashids.org/java) which is fully compatible with the Hashids algorithm, but additionally includes optional configurable features like e.g. exception handling and disabling of the maximum number size limit.

Maybe this can be ported to this library. This way it wouldn't be necessary to release a new major version with breaking changes forcing users to decide to use the then outdated 1.x version(s) if they'd like to keep the current (reference implementation) behavior.

[gabrielhora]

Thank you for explaining @0x3333, that is understandable.

I don't think it's necessary to change the IllegalArgumentException to simply returning an empty string (as you suggested) in 1.1 release (or whatever) just to change it back to exceptions in a future 2.0 release, right?

Maybe we should just keep it like this and solve this inconsistency in the 2.0 release later?

[0x3333]

I believe that we have 2 types of users, the one that are using the library in a pure java system, and the ones that are using in a plural environment, with other implementations, like python, go, etc.

We could workout a version where the default implementation will be more a java idiom, with exceptions, larger numbers, etc, but with a "compatibility" layer, which will "mimic" the de facto implementation(Returning empty strings, limiting input, etc). Or something like the @arcticicestudio did, using HashidsFeature.

In my opinion this library must be fast, lean and simple.

I'll create an issue to track this new version.

[0x3333]

Issue 55 - V2 Proposal