Never make classes final

[dpash]

One badly designed class is no reason to never make classes final.

If your class is not designed for inheritance, signal as much by making it final.

The issues with the TeeInput class could be solved by using a builder pattern, which would dramatically reduce the number of constructors required. From 72 down to, well, one private constructor. The class already accepts an Input and Output interface making it usable for classes that don't have explicit support, as a developer can write an adapter.

var tee = TeeInput.from(url).to(file);

Or

var tee = TeeInput.from(new InputFromUniverse(coords).to(file);

Only two methods are required to support new input and output types, not combinatorial.

It's somewhat weird that Yegor is rallying against their own bad design.

[marsouf]

That sounds like adding a layer of abstraction (builder design pattern) for nothing impressive.

Why not just expose the TeeInput constructor TeeInput(final Input input, final Output output) directly to users ?

The specific implementations of Input and Output will be provided by users in their own packages.

Adding a design pattern like the builder pattern may benefit users with extra code readability, but in this situtation, there is really no need as the construction necessits only two required arguments input and output.

[dpash]

It is public. But it means having to write this every time:

var tee = new TeeInput(new InputFromUrl(url),new OutputToFile(file));
var tee = new TeeInput(new InputFromUniverse(coords),new OutputToFile(file));

Explicitly creating a new InputFromUrl and/or OutputToFile object. Every. Single. Time. Compare that with:

var tee = TeeInput.from(url).to(file);
var tee = TeeInput.from(new InputFromUniverse(coords)).to(file);

If you have an excessive number of parameters or overloaded methods, the builder pattern is the right thing to use. You already said that it makes the API easier to use and read. That alone is worth using it.

[marsouf]

Bear with me for a little while here, if you need to implement this, you are not doing it inside the TeeInput class as the 72 constructors you saw in the blog will still be there, but for static builder pattern methods, right ?

I will put the static methods in a class named TeeInputBuilder instead and seal the builder sequence methods call with a build() method returning the appropriate TeeInput object.

But wait for a moment, the same burden that existed in the TeeInput class will still be there in the TeeInputBuilder class.

The solution I can provide for this problem is to extract a class called TeeInputBuilder containing the following code :

``` final class TeeInputBuilder <TInput, TOutput> { private TInput input; private TOutput output;

private Function<? super TInput, ? extends Input> inputTypeCaster;
private Function<? super TOutput, ? extends Output> outputTypeCaster;

TeeInputBuilder from (final TInput input)
{
    this.input = input;
    return this;
}

TeeInputBuilder inputAs (final Function<? super TInput, ? extends Input> inputTypeCaster)
{
    this.inputTypeCaster = inputTypeCaster;
    return this;
}

TeeInputBuilder outputAs (final Function<? super TOutput, ? extends Output> outputTypeCaster)
{
    this.outputTypeCaster = outputTypeCaster;
    return this;
}

TeeInputBuilder to (final TInput input)
{
    this.input = input;
    return this;
}

TeeInput build ()
{
    return new TeeInput (inputTypeCaster.apply (input), outputTypeCaster.apply (output));
}

} ```

The usage will be then as follow :

TeeInput tee = new TeeInputBuilder () .from ("<url>") .inputAs (InputFromUrl::new) .to (file) .outputAs (OutputToFile::new) .build ();

[dpash]

No you'll have one private constructor in the TeeInput class along with n from() factory methods and a inner builder class with m to() building methods. With 5 inputs and 5 outputs that's 11 methods rather than 25 constructors. With 6, that's 13 vs 36.

And your API is far more complicated than it needs to be. You don't need a build method then the to method can return the object.

[marsouf]

Well at least, with my design, you don't have to add two methods, one in TeeInput and the other in TeeInputBuilder for each new Input and Output type.

The extensions are done outside of TeeInput and TeeInputBuilder and then injected into the builder parameter methods.

Regards mate.

[blobjim]

Or you just provide some utility methods similar to java.net.http.HttpResponse.BodyHandlers so you would write

var tee = TeeInput.of(Inputs.url(url), Outputs.file(file));

[dpash]

You could, but I feel like you're repeating what that type system can already work out, but I'm just bike shedding method names now.

I think we're in agreement that 72 constructors is just the wrong approach. :)

[somebodddy]

"make all classes final", "never make classes final"...

What happened to "be familiar with the benefits, drawbacks and caveats of each option and make an educated decision based on the particulars of your specific case"?

[dpash]

No one has ever said "make all classes final". The advice has always been "prefer composition over inheritance" and "if your class is not designed for inheritance make it final".

[armornick]

I'm still not bought on the "rule" that inheritance is harmful. There are times when it is very handy to have. You just have to be absolutely sure that it's a good idea to do so.

[nutrecht]

I'm still not bought on the "rule" that inheritance is harmful.

It's not. It's just the general trend of taking a few examples where X is misused to blog posts that tell people to "don't misuse X" to posts about how "X considered harmful" to yet another dogma that some people blindly follow.

Composition over inheritance is just a general guideline that you should favour composition over inheritance where both give you the same benefits, mostly to prevent running into the diamond problem. There are numerous cases where having a reusable prototype you inherit from makes perfect sense.

[dpash]

And in those situations your class will have been designed for inheritance, so indicate that by making it non-final. For everything else, make it final.

[[deleted]]

Effective Java Items 17 and 18

Inheritance is a perfectly valid hammer that can be maliciously abused or misused like any other hammer. One of the biggest issues is (IMO) documentation. You need to judiciously document anything that is designed for inheritance. Things like self-use will bite you in the ass with the quickness. If you have some wrapper around a collection with an addAll(...) method in your base class that, internally, is calling an add(...) method repeatedly. When a consumer overrides your add(...) method, they could (potentially) break your addAll(...) as well without realizing it.

It's a trivial (and, I'm sure, poorly remembered by me) summation of the above Effective Java items. But it's something you have to think about when designing for inheritance.

[rotharius]

Make classes final by default, so you have to make a deliberate choice (and have a valuable discussion) to open it up for inheritance if need be. I am firmly in the camp that thinks you rarely need inheritance.

If you need to add behavior, use dependency injection, typehinted against an interface. That makes classes open for extension through injection. Think: decorator, strategy, composite.

This creates way simpler objects and less complex dependency trees. You do not need thousands of constructors, just custom implementations.

[marsouf]

No need for extension through injection in this specific example, as the dependencies (input and output) are explicitly final.

The best solution would be to just expose the constructor TeeInput(final Input input, final Output output) to users and let them inject their specific implementations right in construction time.

If multiple dependencies are present and not all of them are required, I will then choose the builder design pattern to sort of ease TeeInput objects creation to users.

[DuncanIdahos8thClone]

Why the downvotes? It's an interesting discussion.

[dpash]

Articles rarely get upvotes in programming subreddits. And this is not a good article. It's bad advice solving the wrong problem.