Use Subclasses and alternative interface implementations to reduce “future” bugs

One of the little-appreciated consequences of subclassing or alternative implementations of Java interfaces is to reduce or eliminate “future bugs”.

“Future bugs” are bugs that are currently not wrong, but will cause problems in the future. Every conditional ( if, ? :, or switch ) is a “future bug”.

For example, in this simplistic example code, there are multiple checks for this.enableLogging. If new code is added then the developer has to constantly remember to check for the this.enableLogging.

public class Foo {
    private final boolean enableLogging;
    public Foo(boolean enableLogging) { 
        this.enableLogging = boolean enableLogging; }
    
    public void stuff1() {
         .....
         if ( this.enableLogging ) {
             Log.warning("here in stuff1");
         }

         .....
         if ( this.enableLogging ) {
             Log.debug("still here in stuff1");
         }
    }
    public void stuff2() {
         .....
         if ( this.enableLogging ) {
             Log.info("here in stuff2");
         }

         .....
         if ( this.enableLogging ) {
             Log.debug("still here in stuff2");
         }
    }
}

Of course sane developers will say that this is silly code, follow the DRY principle and have something like this:

public class Foo {
    private final boolean enableLogging;
    public Foo(boolean enableLogging) { this.enableLogging = boolean enableLogging; }
    
    public void stuff1() {
         .....
        warning("here in stuff1");
        .....
        debug("still here in stuff1");
        ....
    }
    public void stuff2() {
         .....
         info("here in stuff2");
         .....
         debug("still here in stuff2");
    }
    private void warning(String message) {
        if (this.enableLogging) {
            Log.warning(message);
        }
    }
    private void info(String message) {
        if (this.enableLogging) {
            Log.info(message);
        }
    }
    private void debug(String message) {
        if (this.enableLogging) {
            Log.debug(message);
        }
    }
}

This is good enough for this simplistic example. However, consider a more realistic case with complex conditionals:

public class Foo {
    private final boolean enableCall;
    private final boolean enableEmail;
    public Foo(boolean enableCall, boolean enableEmail) { 
        this.enableCall =  enableCall; 
        this.enableEmail =  enableEmail; }
    
    public void stuff1() {
         .....
         if ( this.enableCall ||  this.enableEmail) {
             doThing1();
         } else if (this.enableEmail || this.bigCheck() ) {
             ... lots of stuff1 lines  .....
        } else {
             .. some different things here....
         }
    }
    public void stuff2() {
         .....
         if ( this.enableCall ||  !this.enableEmail ) {
             doThing1();
         } else if (this.enableEmail || this.bigCheck() ) {
             ... lots of stuff2 lines .....
        } else {
             .. some different things for stuff2() here....
         }
    }
    private void doThing1() { ... }
    private boolean bigCheck() { .... } 
}
...
public class Bar() {
    ....
    Foo foo = new Foo(true, false);
    ....
    Foo foo1 = new Foo(false, true);
}

Do you see the bugs?

  1. Notice that in stuff2() the first conditional is different than the first conditional in stuff1(). Is this a bug? Who knows? ( Lets assume it is )
  2. Because the only users of Foo set either enableCall or enableEmail, the only actual code paths through Foo.stuff1() and Foo.stuff2() result in calls to doThing1()

A developer just inspecting Foo can find neither issue easily.

Subclassing/alternative interface implementations solve this problem. The subclass created decides, at object creation, all the conditionals.

So the above code becomes:

public interface Foo {
    void stuff1();
    void stuff2();
}

//  this.enableCall ||  this.enableEmail case ( The original stuff2() check was wrong )
public class Foo1Impl() implements Foo {
    public void stuff1() { doThing1(); }
    public void stuff2() { doThing1(); }
    private void doThing1() { .... }
}

// !this.enableCall && this.enableEmail case
public class Foo2Impl() implements Foo {
    public void stuff1() { if (bigCheck() ) { ... lots of stuff1 lines  ..... } }
    public void stuff2() { if (bigCheck() ) { ... lots of stuff2 lines  ..... } }
    private boolean bigCheck() { .... } 
}
public class Foo3Impl() implements Foo {
    public void stuff1() { 
          .. some different things for stuff1() here.... }
    public void stuff2() { 
         .. some different things for stuff2() here.... }
}

public class Bar() {
    ....
    Foo foo = new Foo1Impl();
    ....
    Foo foo1 = new Foo1Impl();
}

Note the consequences of this division,

  1. doThing1() can only be called by a Foo1Impl.
  2. bigCheck() is isolated to Foo2Impl
  3. depending on bigCheck()’s implementation, bigCheck() may be could be called once to determine if Foo2Impl or Foo3Impl is created.
  4. There is no question about the stuff2() check being correct or incorrect. ( the conditional no longer exists! )
  5. And it is now obvious that only Foo1Impl’s code path is used. If this is a bug, the bug is now obvious. If not then the Foo2Impl and Foo3Impl code can be eliminated.
This entry was posted in code review. Bookmark the permalink.

Leave a Reply

Your email address will not be published. Required fields are marked *