LocaleId implements equals and compartTo and support a String, but that's error prone

Although the signatures of the equals and compareTo methods take an Object, they check if the object is a String and build a LocaleId from it.

That seems handy, it seems very convenient to do something like this:

   LocaleId locale = ...;
   locale.equals("en");

But it can be confusing:

   locale.equals("en"); // true
   "en".equals(locale); // false

So it looks like a == b, but b != a

Or this surprising behavior:

        assertTrue(LocaleId.ENGLISH.equals("en"));
        assertFalse("en".equals(LocaleId.ENGLISH));

        // This one fails
        assertEquals(
                LocaleId.ENGLISH.equals("en"),
                "en".equals(LocaleId.ENGLISH));

        // Throws java.lang.AssertionError:
        // expected: java.lang.String<en>
        // but was: net.sf.okapi.common.LocaleId<en>
        assertEquals("en", LocaleId.ENGLISH);

        // Throws java.lang.AssertionError:
        // expected: net.sf.okapi.common.LocaleId<en>
        // but was: java.lang.String<en>
        assertEquals(LocaleId.ENGLISH, "en");

Some principles of equals() method (https://www.geeksforgeeks.org/equals-hashcode-methods-java/, but I can probably quote other articles):

Reflexive : for any reference value a, a.equals(a) should return true.
Symmetric : for any reference values a and b, if a.equals(b) should return true then b.equals(a) must return true.
Transitive : for any reference values a, b, and c, if a.equals(b) returns true and b.equals(c) returns true, then a.equals(c) should return true.
Consistent : for any reference values a and b, multiple invocations of a.equals(b) consistently return true or consistently return false, provided no information used in equals comparisons on the object is modified.
Extra note: For any non-null reference value a, a.equals(null) should return false.

Pretty much everybody complains about it: Eclipse, IntelliJ, the Google ErrorProne

Comments (3)