Which is better design: Determining if a function should execute from outside of it, or inside of it?

Question

I'm often presented with these two options. From a design point of view, which of these is subjectively better, and why?

Option A

class foo {
  private boolean bar() {
    //Stuff
  }

  private void A() {
     if(bar())
       B();
  }

  private void B() {
      // more stuff
  }
}

Option B

class foo {
  private boolean bar() {
    //Stuff
  }

  private void A() {
      B();
  }

  private void B() {
     if(!bar())
        return;

     // more stuff
  }
}

Let's assume that code duplication isn't an issue here, and the division is only to separate, logically, the code for better readability/understanding (A, B, and bar have meaningful names in context). Is it more readable to have the reader know that B might not do anything when looking at just A? Or is the encapsulation of state better put into B? Obviously, this can be refactored to separate B and bar() into a separate class entirely, which separates concerns even more, but assuming mostly shared state, which is better?

Answer 1

The important bit is who owns bar? If it's unclear, consider if B can live up to it's method name considering the value of Bar. If so, then make it part of B. If not, then make it part of A.

In a real world scenario, you'll generally find methods that fulfill a design responsibility, and helper methods that fulfill implementation steps. Generally, the decision of whether to call an implementation method resides with the API method - which is generally the public method.

public class Control {
    public bool IsVisible;
    public void Render() {
       if (this.IsVisible) {
          this.PrepareForRender();
       }
    }
    private void PrepareForRender() {
       // just do it. Don't check IsVisible
    }
 }

It's possible that you have two different API methods calling each other. In that case, I'd suggest falling back on the semantics of why doesn't B run when Bar is false? Is it because B is unnecessary - then B should decide.

 public class List {
    public int Count;
    public void SetTo(object[] values) {
       this.Clear(); // need to clear values; don't really care how
    }
    public void Clear() {
       // Count = 0 is just a shortcut to fulfilling Clear's responsibility
       if (this.Count == 0) return; 
    }
 }

Is it because B is incorrect? Then A should decide.

public class File {
    public bool IsBinary;
    public void Write() {
       if (this.IsBinary) {
          this.OpenBinaryStream();
       } else {
          this.OpenTextStream();
       }     
    }
    private void OpenBinaryStream() {
    }
    private void OpenTextStream() {
    }
 }

Because it's actively harmful? A should decide, and B should throw an exception:

public class File {
    public bool UseSecureDelete;
    public void Delete() {
       if (this.UseSecureDelete) {
          this.ZeroBytes();
       }
    }
    private void ZeroBytes() {
       if (!this.UseSecureDelete) throw new InvalidOperationException();
    }
 }

Answer 2

I strongly disagree with @jmquigley.

Whatever "performance gain" is in-play here is minuscule and negligible.

The "if" condition should reside in the member in which it's logically (or even semantically) a part of.

Since you haven't clued us in to what the actual object or member functions are, only you can make this decision.

Answer 3

It depends on whether foo() and B() lie on the same level of abstraction.

If they do, then Option A is preferable.

Otherwise (foo() is an implementation detail of B() and therefore lower than B()) Option B is better.

So, the guiding principle here is: don't mix levels of abstraction.

Answer 4

Shortest code wins. You don't want to have multiple appearances of:

 if(!bar.contains(baz))
   B(baz, stuff);

If you NEVER want to add something to bar that is already present, the second implementation is clearly superior.

However, in almost all modern languages, you could make bar a set, and then say something like this:

if (bar.add(baz))
  // more stuff

Answer 5

I suggest you consult with the mythical rubber duck of the "Rubber duck debugging" fame. Don't be fooled by his debugging powers because he is also adept at figuring out design issues.