Java properties: shadowing vs. exposing

gistfile1.java

class BaseClass {

  private final Dependency dependency;
  …

  public MyBaseClass(Dependency dependency) {
    Assert.notNull(dependency);
    this.dependency = dependency;
  }

  protected void myBusinessMethod() { … }
}

class SubClass extends BaseClass {

  private final Dependency dependency;

  public SubClass(Dependency dependency) {
    super(dependency);
    this.dependency = dependency;
  }

  public void method() {
    … = this.dependency; // using local dependencies
    … = myBusinessMethod(); // using functionality of the superclass
  }
}

// VS

class BaseClass {

  private final Dependency dependency;
  …

  public MyBaseClass(Dependency dependency) {
    Assert.notNull(dependency);
    this.dependency = dependency;
  }

  protected Dependency getDependency() {
    return dependency;
  }

  protected void myBusinessMethod() { … }
}

class SubClass extends BaseClass {

  public SubClass(Dependency dependency) {
    super(dependency);
  }

  public void method() {
    … = getDependency(); // accessing internals of the superclass
    … = myBusinessMethod(); // using functionality of the superclass
  }
}

I prefer the former approach for the following reasons:

• all the components the sub-class interacts are local to the class
• the coupling to the superclass is lower as we're not affected if the accessor method is changed or removed

I generally use the latter approach as it's slightly more concise (from the subclass's perspective). I'd declare the accessor final, though.

I go for the later as above for conciseness.

One thing to add maybe: assume the two classes are not part of the same codebase. So you'd essentially start implementing the sub-class in your own codebase referencing the superclass in a library. The reason I think this influences the decision is that in the former approach you essentially have less assumptions about the sub class. You essentially don't need to know whether the superclass keeps the constructor argument inside a field. It could simply use it to do stuff in the constructor and drop the reference.

If we follow the second approach you face another issue: where do you draw the line of what to expose to sub-classes? Do you add a protected getter for all of the properties? Do you maybe even have protected properties only? Doing so you essentially expose all the internals of the superclass where it is actually meant to provide some convenience methods in the first place. So I think there's a difference in the abstraction level here as well (exposing functionality VS. exposing internals + functionality).

Another aspect to the issue: assuming we would use delegation over inheritance, you would also declare the dependency locally:

class Delegate {

  private final Dependency dependency;

  public Delegate(Dependency dependency) {
    Assert.notNull(dependency);
    this.dependency = dependency;
  }

  public void myBusinessMethod() { … }
}

class Client {

  private final Delegate delegate;
  private final Dependency dependency;

  public Client(Delegate delegate, Dependency dependency) {
    Assert.notNull(delegate);
    Assert.notNull(dependency);
    this.delegate = delegate;
    this.dependency = dependency;
  }

  public void clientMethod() {
    delegate.myBusinessMethod();
    dependency.methodCall();
  }
}

// VS

class Delegate {

  private final Dependency dependency;

  public Delegate(Dependency dependency) {
    Assert.notNull(dependency);
    this.dependency = dependency;
  }

  public void myBusinessMethod() { … }

  public Dependency getDependency() { … }
}

class Client {

  private final Delegate delegate;

  public Client(Delegate delegate) {
    Assert.notNull(delegate);
    this.delegate = delegate;
  }

  public void clientMethod() {
    delegate.myBusinessMethod();
    delegate.getDependency().methodCall();
  }
}

In this case it get's even more obvious how one is exposing internals of the delegate (formerly superclass) and how this negatively impacts the design. We're violating the law of demeter and rely on component internals being exposed. Assume interfaces being introduced for Delegate. Why should the interface contain a method exposing internals? What if an interface doesn't actually work with a Dependency?

well not sure what your real goal is.

In theory you always find cases you can't manage (without bytecode instrumentation). A rule i already saw/used is "if i know i can be extended i use protected attributes" otherwise i suppose i'll not be and if somebody needs he will copy/paste my code

So that would mean that for any non-final class you'd have all properties protected? Why have encapsulation at all then as you expose all internals anyway?

I tend to use the approach in the second BaseClass to expose a method. This allows me to control what is allowed and it establishes a contract for extenders, a contract that shouldn't be broken without a very good reason. Besides that, accessing the internals of a superclass feels hacky and dirty unless it's the only way (i.e., a class that wasn't designed for extension).

Another advantage of exposing an accessor method, rather than relying on the subclass shadowing the dependency, is that it means that the superclass can wrap the dependency if it needs to. Doing so means that the subclass will then use the wrapped dependency, whereas in the shadowing approach it'll be using the unwrapped version. That might not be a requirement when you first write the superclass, but it helps to keep your options open.

With respect to where you draw the line; like Bruce, I'd only provide protected final getters for the pieces of the superclass that I consider to be part of its contract. I definitely wouldn't expose all of its fields via protected getters.