0

In Dependency Injection, field injection is generally discouraged in favor of constructor argument injection so that one can make the injected field final (immutable). Constructor argument injection also offers a few other conveniences. However, if an injected field is private, or even protected, needing to create a constructor in the subclass feeding a value up into the super class that is otherwise should not be the concern of the subclass, runs contrary to the the beauty of inheritance - a subclass getting the provisions of the parent class. Assuming that there is no subclass-specific value for the field in question, every single subclass would have to just repeat the same constructor code. This can make the code more complex and harder to understand, especially when there are multiple dependencies and multiple levels of inheritance.

public class A {
  private final Foo foo;

  public A(Foo foo) {
    this.foo = foo;
  }
}

public class B extends A {
  @Autowired
  public B(Foo foo) {
    super(foo);
  }
}

The immutability in the above mainly refers to the prevention of inadvertent reassignment and not necessarily any performance optimization that the final qualifier can impart to the field. In spite of this, some insist that all injected fields must be made final. What are the other pros and cons?

Madhu
  • 1
  • 1
  • `However, if an injected field is private, or even protected, needing to create a constructor in the subclass feeding a value up into the super class, runs contrary to the the beauty of inheritance` Can you justify this statement? What actual arguments do you have that support the idea that constructors are "contrary" to some kind of "beauty"? I'm asking because I don't understand. If you remove the `@Autowire` annotation above, what you have is perfectly normal Java inheritance tree, and all of those elements are required, none can be omitted. – markspace Aug 06 '23 at 23:21
  • 1
    `Immutability: A private field is already immutable` **NO** Java has a specific definition of immutability, you don't get to pick your own. The JVM needs to understand what you are doing or you do *NOT* get an immutable object. Go read the spec: https://docs.oracle.com/javase/specs/jls/se20/html/jls-17.html#jls-17.5 – markspace Aug 06 '23 at 23:22
  • 1
    And I think points 2 through 4 are really weak, you should justify those much better imo. – markspace Aug 06 '23 at 23:25
  • Don't sweat it. If you can write a good unit test fro your class, it's a good class. I don't know why you need a constructor in `A` in this case, but it's ok too. Doesn't matter what it's called as long as it works. – Dima Aug 06 '23 at 23:42
  • 1. Having no shared mutable state allows you to quickly see that a class is thread safe, and final fields are an important part of that. 2. You need to explain better what you mean. Usually a class which is extended would be `abstract`, so never created. 3. What increased complexity do you mean? The only difference is a constructor. 4. As syllogistyc suggests, you can use composition to avoid coupling. If the subclass is overriding methods, then it is already tightly coupled. – tgdavies Aug 07 '23 at 00:49
  • As a general rule, using words like 'beauty', or making statements like 'more complex' without explaining how or defining what such a complex (ha!) word as 'complex' means - is a big mistake. Programming isn't like painting. It's like bridge building. It works, or it falls. It was built efficiently, or it went way over budget and is a maintenance nightmare. Yes, a bridge __can__ be a work of art, and so can some code - but if you local bridge builder harps on and on about how pretty the bridges are and goes 'wha?' when you ask about safety - run for the hills. – rzwitserloot Aug 07 '23 at 01:44
  • Imagine you tried to be a biologist and you preferred one explanation (let's say, that the humours of the body being out of whack causes disease) because you find this more 'elegant' than an explanation with viruses but also bacteria and even parasites. It's idiotic. Don't apply it to programming, either. – rzwitserloot Aug 07 '23 at 01:45
  • Presumably this is why SO has a rule against precisely this kind of question. It just pits my innate sense of aesthetics against yours. It's as useful as me telling you: "I find this painting quite pretty" and you going: "What?? It's an ugly mess!" - that's not a debate. That's just pointless yelling. – rzwitserloot Aug 07 '23 at 01:46

1 Answers1

2

The point of a subclass is to create different, specialised versions of the parent and should be reserved for an "IS-A" relationship. You've probably heard "always prefer composition over inheritance".

With that in mind, class B still needs an input Foo for all the same reasons that class A needs one - by merit of B "being" an A ("is-a").

In answer to your points:

  1. Immutability is a best practice to help you or other developers avoid errors. Same motivations as using non-nullable types. And it's self-documenting. Imagine coming across a reasonably complex class written by someone else - you don't know if that field is intended to be mutated.
  2. Inversion of Control is actually part of the point. That's why DI is used at all (see https://stackoverflow.com/a/3140/8409571). If A breaks because B gave it a different dependency, that suggests other problems with your code.
  3. Many layers of dependency can easily get complex if not designed carefully. Some languages are more verbose than others (Java). There are other ways to get around this problem e.g. Builders or Options objects. If those constructor args seem unnecessary, then you might have misused inheritance.
  4. Tight Coupling: Yes. Exactly. B is an A. It inherits all of A's baggage. Though it should only be one-way coupling B -> A.

For all these reasons, you need to implement inheritance carefully. Kotlin, for example, defaults everything to final unless you specify open.

TL;DR

If B wants to reuse code from A but shouldn't have to worry about Foo, then use Composition instead:

public class A {
  private final Foo foo;

  public A(Foo foo) {
    this.foo = foo;
  }
}

public class B {
  private final A a

  public B(A a) {
    this.a = a
  }
}
syllogistyc
  • 41
  • 1
  • 3