CODEWITHSUNDEEP
javaconditional-operator
user1158745
user1158745

Reputation: 2490

java Expression too complex reduce the number of conditionl operators

We have a program that we run against our code to adhere to some coding standards.

The program says:

Expression should not be too complex, reduce the number of conditional operators used int he expression Min allowed 3.

How can I reduce the number of conditional operators? perhaps put the keyevents in an array?

public boolean onlyNumbers(KeyEvent evt) {
    char c = evt.getKeyChar();
    boolean returnValue = true;
    if (
        !(
            Character.isDigit(c) 
            || c == KeyEvent.VK_BACK_SPACE
            || c == KeyEvent.VK_DELETE 
            || c == KeyEvent.VK_END 
            || c == KeyEvent.VK_HOME
        )
        || c == KeyEvent.VK_PAGE_UP
        || c == KeyEvent.VK_PAGE_DOWN
        || c == KeyEvent.VK_INSERT
    ) {
        evt.consume();
        returnValue = false;
    }
    return returnValue;
}

Upvotes: 5

Views: 6041

Answers (4)

Zéychin
Zéychin

Reputation: 4205

Here is one way to do it. Could be improved by using an array.

public boolean onlyNumbers(KeyEvent evt) {
    char c = evt.getKeyChar();
    boolean returnValue;

    returnValue =  !(Character.isDigit(c));
    returnValue &= !(c == KeyEvent.VK_BACK_SPACE);
    returnValue &= !(c == KeyEvent.VK_DELETE);
    returnValue &= !(c == KeyEvent.VK_END);
    returnValue &= !(c == KeyEvent.VK_HOME);
    returnValue |= (c == KeyEvent.VK_PAGE_UP);
    returnValue |= (c == KeyEvent.VK_PAGE_DOWN);
    returnValue |= (c == KeyEvent.VK_INSERT);
    if(returnValue)
    {
        evt.consume();
        returnValue = !returnValue;
    }
    return returnValue;
}

The first five and last three assignments could be compacted in their respective groupings to two total assignments, but that will boil down to your coding standards.

Upvotes: 0

Dima
Dima

Reputation: 40510

final String junkChars = new String(new char[] {
    KeyEvent.VK_BACK_SPACE,
    KeyEvent.VK_DELETE,
    KeyEvent.VK_END,
    KeyEvent.VK_HOME
    /* The last three seem unused in the latest version
    KeyEvent.VK_PAGE_UP,
    KeyEvent.VK_PAGE_DOWN,
    KeyEvent.VK_INSERT */
});
if(!Character.isDigit(c) && junkChars.indexOf(c) == -1) {
   evt.consume();
   return false;
}  else {
    return true;
}

Upvotes: 5

OldCurmudgeon
OldCurmudgeon

Reputation: 65879

A strict refactor of your sample would look like:

public boolean onlyNumbers(KeyEvent evt) {
    char c = evt.getKeyChar();
    boolean returnValue = true;
    boolean bad = Character.isDigit(c);
    bad |= (c == KeyEvent.VK_BACK_SPACE);
    bad |= (c == KeyEvent.VK_DELETE);
    bad |= (c == KeyEvent.VK_END);
    bad |= (c == KeyEvent.VK_HOME);
    boolean good = (c == KeyEvent.VK_PAGE_UP);
    good |= c == KeyEvent.VK_PAGE_DOWN;
    good |= c == KeyEvent.VK_INSERT;
    if (!bad || good) {
        evt.consume();
        returnValue = false;
    }
    return returnValue;
}

This highlights the concerns others have noted about the placing of your brackets

Upvotes: 2

chiastic-security
chiastic-security

Reputation: 20520

One way you could do this is to construct a HashSet<Character> keys containing all the characters you want to test for, and then use keys.contains(c) to test whether it's one of them.

Alternatively, you could use a switch statement, and let all those characters fall through to the same block of code.

But my personal favourite would be to ignore the warning. The code is perfectly clear as it is (modulo ajb's comment about parentheses).

Upvotes: 0

Related Questions