HiveBrain v1.2.0
Get Started
← Back to all entries
principlejavaModerate

Indented if/else approach

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
indentedapproachelse

Problem

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;
    }

Solution

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.

Code Snippets

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;
}

Context

StackExchange Code Review Q#5453, answer score: 10

Revisions (0)

No revisions yet.