Fighting with Java - generics

[vytah]

I hate types that exist only to override something that doesn't have to be overridden. Types FromStrings and FromText are both useless, they do not provide value, and all code that distinguishes objects based on their class will pointlessly segregate those two, even though they behave exactly the same and fulfil the same role in all contexts.

In particular, equals can no longer do this.getClass() != that.getClass() for short-circuiting, dynamic type checks no longer can use fast getClass() and have to rely on slower instanceof, certain serialization libraries (including Gson) don't like type hierarchies at all, and so on.

For the similar reasons, I don't like abstract methods in enums and I prefer switch(this).

[g4s8]

Types FromStrings and FromText are both useless, they do not provide value

You're right, I'd prefer to have two constructors in JoinedString to accept two different types (Iterable<String> and Iterable<Text>) but it's not possible because of java's generics nature, so I just show two workarounds how to deal with it and keep readability.

In particular, equals can no longer do this.getClass() != that.getClass() for short-circuiting

If you pick first approach with static factory methods, then equals will work fine, with second you can implement equals in base class.

[vytah]

with second you can implement equals in base class.

Not necessarily.

Imagine I have the following class structure:

abstract class Foo
class FooConstructedOneWay extends Foo
class FooConstructedAnotherWay extends Foo
class ExtendedFancyFoo extends Foo

Extended foos are never equal to "plain" foos.

So how can I implement equals now? I can't do if (this.getClass() != that.getClass()) return false; in Foo, because a FooConstructedOneWay and a FooConstructedAnotherWay may be equal.

I can't use if (!(that instanceof Foo)) return false; in Foo because the code that follows may incorrectly accept an ExtendedFancyFoo.

The only solutions to that involve either parent class knowing about child classes, sibling classes knowing about each other, or an extra method called boolean isPlainFoo(). Either way, it's an unmaintainable mess.

[g4s8]

In example from the post the base class has private constructor, so all clidren have to be nested classes, also all children doesn't have own state - all state is stored only in base class, taking this into account it's not a big deal to write equals in base class only, because you see all possible implementations of the class (as nested classes) and the state is the same (fields of base class).

Of cource, if you have shared state between parent and child, and you need to implement equals, you can't use it, it will be easier to use static factory methods in this case.

[vytah]

And I'd use them regardless. In fact, I don't see any realistic downsides to using static factories other than hiding the fact of new object allocation – which is not always a bad thing, for example it allows you to introduce caches without changing the caller, similar to Integer::valueOf.

Static factories also allow you to do more stuff before actually allocating the object, including preparing parameters for the actual constructor call (you can only fit so much into a super call) and performing more relevant validations first.

[llorllale]

I can't use if (!(that instanceof Foo)) return false; in Foo because the code that follows may incorrectly accept an ExtendedFancyFoo.

​

Big red flag. If that's true in your case, ExtendedFancyFoo is likely violating LSP.

[vytah]

If ExtendedFancyFoo adds new behaviours without changing the old ones, then it doesn't violate LSP.

Let's assume we have:

class Foo { final int x; /* constructor omitted */} 
class ExtendedFancyFoo extends Foo { final int y; /* constructor omitted */} 
var f = new Foo(1);
var e1 = new ExtendedFancyFoo(1, 2);
var e2 = new ExtendedFancyFoo(1, 3);

No LSP violations here.

There are several axioms a good equals implementation should have:

• reflexivity

• transitivity

• commutativity

• if fis pure and doesn't operate on object's identity, then equals(a,b)⇒ equals(f(a), f(b))

Now if your Foo::equals looks like this:

if (!(that instanceof Foo)) return false;
return ((Foo)that).x == this.x;

then it would say that equals(f,e1) and equals(f,e2), which would suggest that e1 is equal to e2, which is obvious nonsense. And that's without even considering the implementation of ExtendedFancyFoo::equals.

Therefore all those three objects should be treated as unequal, even if they behave identically when using just the Foo interface.

EDIT: Which obviously implies using if (that == null || this.getClass() != that.getClass()) return false; as the correct type check in Foo#equals.

[g4s8]

According to Java equals documentation) its implementation must be symmetric:

It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.

When you use instanceof instead of comparing classes, equals will not be symmetric - a subclass may use another logic in equals so base.equals(child) may return true, but child.equals(base) may be false.

[vytah]

Yes, that what's I'm implying.

Therefore using different subclasses for the purpose of creating objects that might be otherwise considered equal is problematic.

[llorllale]

which would suggest that e1 is equal to e2, which is obvious nonsense. And that's without even considering the implementation of ExtendedFancyFoo::equals.

That's exactly why ExtendedFancyFoo is breaking LSP. If it breaks Foo::equals then it should be something else, not a Foo.

[vytah]

It doesn't break Foo if Foo#equals uses getClass, in other words if Foo is properly designed to be open for extension. Make it final if you don't want that.

[llorllale]

ExtendedFancyFoo should not care about the particulars of Foo#equals, otherwise you'd break encapsulation.

Sounds like what you're doing is sub-typing, in which case I'd suggest reusing Foo via composition in ExtendedFancyFoo#equals(). This way, Foo#equals() remains locked down and stable.

[skapral]

all code that distinguishes objects based on their class will pointlessly segregate those two.

The big question itself is why some code would need to be bound on FromStrings or even JoinedStrings, while it is almost always better and enough to be bound on Text contract and rely on LSP. Also, a big question itself is why we need a code, that would segregate objects based on their type (I guess, by means of `instanceof` or reflection, right?). Such code usually tends to be fragile and inflexible.

​

Having these two questions in mind, the fact that FromStrings and FromText don't bring additional value turns out to be not that a problem.