CODEWITHSUNDEEP
javaif-statementternary-operatorshort
userit1985
userit1985

Reputation: 993

Short form for Java If Else statement

I have a method which checks for nulls. Is there a way to reduce the number of lines in the method? Currently, the code looks "dirty":

private int similarityCount (String one, String two) {

    if (one == null && two == null) {
        return 1;
    } else if (one == null && two != null) {
        return 2;
    } else if (one != null && two == null) {
        return 3;
    } else {
        if(isMatch(one, two))
             return 4;
        return 5;
    }

}

Upvotes: 28

Views: 10037

Answers (9)

Manh Le
Manh Le

Reputation: 1650

private int similarityCount (String one, String two) {

    if (one == null && two == null) {
        return 1;
    } 

    if (one == null) {
        return 2;
    } 

    if (two == null) {
        return 3;
    } 

    if (isMatch(one, two)) {
        return 4;
    }
    return 5;
}

Upvotes: 61

user7507190
user7507190

Reputation: 1

This should be slightly faster if neither is null, as it only performs one 'if' statement in this case.

private int similarityCount (String one, String two) {

    if (one == null || two == null) {  // Something is null
        if (two != null) { // Only one is null
            return 2;
        }

        if (one != null) { // Only two is null
            return 3;
        }

        return 1; // Both must be null
    } 

    return isMatch(one, two) ? 4 : 5;
}

Upvotes: 0

Dancrumb
Dancrumb

Reputation: 27529

You can create a pseudo-lookup-table. Some people frown on the nested ternary operators and it's highly dependent on whitespace for readability, but it can be a very readable approach to conditional returning:

private int similarityCount (String one, String two) {
    return (one == null && two == null) ? 1
         : (one == null && two != null) ? 2
         : (one != null && two == null) ? 3
         : isMatch(one, two)            ? 4
         :                                5;
}

Upvotes: 5

Erik Madsen
Erik Madsen

Reputation: 2014

I like expressions.

private static int similarityCount (String one, String two) {    
    return one == null ? 
        similarityCountByTwoOnly(two) : 
        two == null ? 3 : (isMatch(one, two) ? 4 : 5)
    ;
}

private static int similarityCountByTwoOnly(String two) {
    return two == null ? 1 : 2;
}

As an aside, I would probably challenge why you are doing this. I would assume you would do some kind of check on the returned integer after you've evaluated it and branch your logic based on it. If that is the case, you've just made a less readable check for null where the user of your method needs to understand the contract implicit in the value of the integer.

Also, here's a simple solution for when you need to check if strings are equal when they may be null:

boolean same = one == null ? two == null : one.equals(two);

Upvotes: 1

Mark Chorley
Mark Chorley

Reputation: 2107

Getting rid of IF statements is good fun. Using a Map is one way of doing this. It doesn't fit this case exactly because of the call to isMatch, but I offer it as an alternative which cuts the similarityCount method body to a single line with one IF

The following code has two IFs. If GetOrDefault didn't evaluate the second argument, it could be reduced to one. Unfortunately it does so the null check inside isMatch is necessary.

You could go a lot further with this if you wanted to. For example, isMatch could return 4 or 5 rather than a boolean which would help you simplify further. 

import com.google.common.collect.ImmutableMap;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;

import java.util.Map;

public class SimilarityCount {

    private Map<SimilarityCountKey, Integer> rtn = ImmutableMap.of(new SimilarityCountKey(null, null), 1, new SimilarityCountKey(null, ""), 2, new SimilarityCountKey("", null), 3);

    public int similarityCount(String one, String two) {
        return rtn.getOrDefault(new SimilarityCountKey(one, two), isMatch(one, two) ? 4 : 5);
    }

    private boolean isMatch(String one, String two) {
        if (one == null || two == null) {
            return false;
        }
        return one.equals(two);
    }

    private class SimilarityCountKey {
        private final boolean one;
        private final boolean two;

        public SimilarityCountKey(String one, String two) {
            this.one = one == null;
            this.two = two == null;
        }

        @Override
        public boolean equals(Object obj) {
            return EqualsBuilder.reflectionEquals(this, obj);
        }

        @Override
        public int hashCode() {
            return HashCodeBuilder.reflectionHashCode(this);
        }
    }
}

In case anyone else fancies a crack at another solution, here are some tests to help you get started

 import org.junit.Assert;
import org.junit.Test;

import static org.hamcrest.CoreMatchers.is;

public class SimilarityCountTest {

    @Test
    public void one(){
        Assert.assertThat(new SimilarityCount().similarityCount(null,null), is(1));
    }

    @Test
    public void two(){
        Assert.assertThat(new SimilarityCount().similarityCount(null,""), is(2));
    }

    @Test
    public void three(){
        Assert.assertThat(new SimilarityCount().similarityCount("",null), is(3));
    }

    @Test
    public void four(){
        Assert.assertThat(new SimilarityCount().similarityCount("",""), is(4));
    }

    @Test
    public void five(){
        Assert.assertThat(new SimilarityCount().similarityCount("a","b"), is(5));
    }

}

Upvotes: 0

Konrad Rudolph
Konrad Rudolph

Reputation: 545558

Since the actual purpose of the function seems to be to handle non-null objects by matching them, I’d handle all the null checks in a guard statement at the beginning.

Then, once you’ve established that no argument is null, you can handle the actual logic:

private int similarityCount(String a, String b) {
    if (a == null || b == null) {
        return a == b ? 1 : a == null ? 2 : 3;
    }

    return isMatch(a, b) ? 4 : 5;
}

This is both more concise and more readable than the other options.

That said, real functions wouldn’t usually return such numeric codes. Unless your method was simplified to exemplify the problem, I’d strongly urge you to reconsider the logic and instead write something akin to what follows:

private boolean similarityCount(String a, String b) {
    if (a == null || b == null) {
        throw new NullPointerException();
    }

    return isMatch(a, b);
}

Or:

private boolean similarityCount(String a, String b) {
    if (a == null) {
        throw new IllegalArgumentException("a");
    }
    if (b == null) {
        throw new IllegalArgumentException("b");
    }

    return isMatch(a, b);
}

These approaches would be more conventional. On the flip side, they may trigger an exception. We can avoid this by returning a java.util.Optional<Boolean> in Java 8:

private Optional<Boolean> similarityCount(String a, String b) {
    if (a == null || b == null) {
        return Optional.empty();
    }

    return Optional.of(isMatch(a, b));
}

At first glance this may seem to be no better than returning null but optionals are in fact far superior.

Upvotes: 25

SachinSarawgi
SachinSarawgi

Reputation: 2692

It can be done in one line using Java conditional operator:

return (one==null?(two==null?1:2):(two==null?3:(isMatch(one,two)?4:5)));

Upvotes: 5

Eran
Eran

Reputation: 393781

I prefer nested conditions in such cases :

private int similarityCount (String one, String two) {
    if (one==null) {
        if (two==null) {
            return 1;
        } else {
            return 2;
        }
    } else {
        if (two==null) {
            return 3;
        } else {
            return isMatch(one, two) ? 4 : 5;
        }
    }
}

Of course you can achieve a shorter version by using more ternary conditional operators.

private int similarityCount (String one, String two) {  
    if (one==null) {
        return (two==null) ? 1 : 2;
    } else {
        return (two==null) ? 3 : isMatch(one, two) ? 4 : 5;
    }
}

Or even (now this is getting less readable) :

private int similarityCount (String one, String two) {  
    return (one==null) ? ((two==null) ? 1 : 2) : ((two==null) ? 3 : isMatch(one, two) ? 4 : 5);
}

Upvotes: 33

default locale
default locale

Reputation: 13436

The code looks clear enough for me. You can make it shorter with nesting and ternary operators:

if(one==null) {
    return two==null ? 1 : 2;
}
if(two==null) {
    return 3;
} 
return isMatch(one,two) ? 4 : 5;

Upvotes: 9

Related Questions