I have a history in programming, but not much in software development. I'm currently writing a piece of software for the company I work at, and I've come to challenge myself on the readability of my code.
I want to know whether this is a "valid" alternative to embedded if statements, or if there is anything better I could use.
Let's say I have the following method:
public void someMethod()
{
    if (some condition)
    {
        if (some condition 2)
        {
            if (some condition 3)
            {
                // ...etc all the way until:
                doSomething();
            }
            else
            {
                System.err.println("Specific Condition 3 Error");
            }
        }
        else
        {
            System.err.println("Specific Condition 2 Error");
        }
    }
    else
    {
        System.err.println("Specific Condition 1 Error");
    }
}
Now the first thing I should point out is that in this instance, combining the conditions (with &&) isn't possible, since each one has a unique error that I want to report, and if I combined them I wouldn't be able to do that (or would I?). The second thing I should point out before anyone screams "SWITCH STATEMENT!" at me is that not all of these conditions can be handled by a switch statement; some are Object specific method calls, some are integer comparisons, etc.
That said, is the following a valid way of making the above code more readable, or is there a better way of doing it?
public void someMethod()
{
    if (!some condition)
    {
        System.err.println("Specific Condition 1 Error");
        return;
    }
    if (!some condition 2)
    {
        System.err.println("Specific Condition 2 Error");
        return;
    }
    if (!some condition 3)
    {
        System.err.println("Specific Condition 3 Error");
        return;
    }
    doSomething();
}
So basically, instead of checking for conditions and reporting errors in else blocks, we check for the inverse of the condition and return if it is true. The result should be the same, but is there a better way of handling this?
Your second approach is fairly good. If you want something a little more baroque, you can move your conditions into Callable objects. Each object can also be provided with a way of handling errors. This lets you write an arbitrarily long series of tests without sacrificing functionality.
class Test {
    private final Callable<Boolean> test;
    private final Runnable errorHandler;
    public Test(Callable<Boolean> test, Runnable handler) {
        this.test = test;
        errorHandler = handler;
    }
    public boolean runTest() {
        if (test.call()) {
            return true;
        }
        errorHandler.run();
        return false;
    }
}
You could then organize your code as follows:
ArrayList<Test> tests;
public void someMethod() {
    for (Test test : tests) {
        if (!test.runTest()) {
            return;
        }
    }
    doSomething();
}
EDIT
Here's a more general version of the above. It should handle almost any case of this type.
public class Condition {
    private final Callable<Boolean> test;
    private final Runnable passHandler;
    private final Runnable failHandler;
    public Condition(Callable<Boolean> test,
            Runnable passHandler, Runnable failHandler)
    {
        this.test = test;
        this.passHandler = passHandler;
        this.failHandler = failHandler;
    }
    public boolean check() {
        if (test.call()) {
            if (passHandler != null) {
                passHandler.run();
            }
            return true;
        }
        if (errorHandler != null) {
            errorHandler.run();
        }
        return false;
    }
}
public class ConditionalAction {
    private final ArrayList<Condition> conditions;
    private final Runnable action;
    public ConditionalAction(ArrayList<Condition> conditions,
            Runnable action)
    {
        this.conditions = conditions;
        this.action = action;
    }
    public boolean attemptAction() {
    for (Condition condition : conditions) {
        if (!condition.check()) {
            return false;
        }
    }
    action.run();
    return true;
    }
}
One might be tempted to add some sort of generic data that could be passed around to share info or collect results. Rather than doing that, I'd recommend implementing such data sharing within the objects that implement the conditions and action, and leave this structure as is.
If I was being particularly pedantic I would use something like this.
boolean c1, c2, c3;
public void someMethod() {
  boolean ok = true;
  String err = "";
  if (ok && !(ok &= c1)) {
    err = "Specific Condition 1 Error";
  }
  if (ok && !(ok &= c2)) {
    err = "Specific Condition 2 Error";
  }
  if (ok && !(ok &= c3)) {
    err = "Specific Condition 3 Error";
  }
  if ( ok ) {
    doSomething();
  } else {
    System.out.print(err);
  }
}
You are now single-exit AND flat.
Added
If &= is difficult for you, use something like:
  if (ok && !c3) {
    err = "Specific Condition 3 Error";
    ok = false;
  }
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With