5
\$\begingroup\$

I have a flow where if a certain condition is true then I should take another approach. That condition can be triggered by different scenarios. But each condition test must be done only if the last one failed. Once a condition is true there is no further logical tests to do. My question is how to do that in an elegant way, the way I'm doing it right now requires an indentation for each condition.

    StatusCode statusCode = null;
    boolean invokeService = true;
    CardPort cardPort = findCardPort(card, port);
    if (cardPort == null) {
        statusCode = StatusCode.CARD_PORT_NOT_FOUND;
    } else {
        Sim sim = cardPort.getSim();
        if (sim == null) {
            statusCode = StatusCode.CARD_WITHOUT_SIM;
        } else {
            User user = sim.getUser();
            if (user == null) {
                statusCode = StatusCode.SIM_WITHOUT_USER;
            }
        }
    }

    if (statusCode != null) {
        invokeService = false;
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ It looks like we're missing critical surrounding code. If not, it should be fairly easy to move the code into a function that can simply return the appropriate status code when an operation fails and not have to nest anything. \$\endgroup\$ – user786653 Oct 18 '11 at 18:24
10
\$\begingroup\$

Personally, I would separate this into multiple functions:

public StatusCode GetStatus( ... )
{
    CardPort cardPort = findCardPort(card, port);
    if (cardPort == null) {
        return StatusCode.CARD_PORT_NOT_FOUND;
    } 

    Sim sim = cardPort.getSim();
    if (sim == null) {
        return StatusCode.CARD_WITHOUT_SIM;
    } 

    User user = sim.getUser();
    if (user == null) {
        return StatusCode.SIM_WITHOUT_USER;
    }

    return null; /* IMO, should be StatusCode.OK instead of null */
}

public bool ShouldInvokeService( ... )
{
    return GetStatus( ... ) == null;
}

Now you don't need the nested indentations to track the status state -- just check conditions until you either know everything is okay, or you run into a problem.

\$\endgroup\$
  • 1
    \$\begingroup\$ Yes. Some people frown upon multiple returns in a method, but I find this approach (on handling error conditions, mostly) much cleaner, indeed. \$\endgroup\$ – PhiLho Oct 20 '11 at 15:56
  • \$\begingroup\$ I'd replace return with throw. \$\endgroup\$ – Allon Guralnek Oct 21 '11 at 18:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.