开发者

Optimizing nested if/ then, java

This code's only redeeming quality is that it works. Can you please help me structure it better?

if (profile.isIgnoreCase()) {
    // ignore case
    if (masterKey.equalsIgnoreCase((targetKey))) {
    if (masterValue.equalsIgnoreCase(targetValue)) {
        doOK(masterKey, masterValue);
        break;
    } else {
        // Key is either Missing or is an Error
        if (checkErrors) {
        doError(masterKey, masterValue, targetValue);
        break;
开发者_JAVA百科        }
    }
    }
} else {
    if (masterKey.equals(targetKey)) {
    if (masterValue.equals(targetValue)) {
        doOK(masterKey, masterValue);
        break;
    } else {
        if (checkErrors) {
        doError(masterKey, masterValue, targetValue);
        break;
        }
    }
    }
}


You can remove some of the repetition by using:

if (profile.isIgnoreCase()) {
    masterKey = masterKey.toLowerCase();
    masterValue = masterValue.toLowerCase();
}

if (masterKey.equals(targetKey)) {
    if (masterValue.equals(targetValue)) {
        doOK(masterKey, masterValue);
    } else {
        if (checkErrors) {
            doError(masterKey, masterValue, targetValue);
        }
    }
}

I have also removed the breaks as I doesn't look like you need them to me

[UPDATE] Alternatively, how about writing a new method to handle the comparison

public boolean isEqual(String a, String b, boolean ignoreCase) {
    if (ignoreCase) {
        return a.equalsIgnoreCase(b);
    } else {
        return a.equals(b);
    }
}

you would then update your code like so:

if (isEqual(masterKey,targetKey,profile.isIgnoreCase())) {
    if (isEqual(masterValue,targetValue,profile.isIgnoreCase())) {
        doOK(masterKey, masterValue);
    } else {
        if (checkErrors) {
            doError(masterKey, masterValue, targetValue);
        }
    }
}


Pull out the key and value comparisons into local variables and you can eliminate the duplicated logic. This avoids modifying the strings, and as a bonus makes the if statements a bit easier on the eyes.

boolean keysMatch, valuesMatch;

if (profile.isIgnoreCase()) {
    keysMatch   = masterKey  .equalsIgnoreCase(targetKey);
    valuesMatch = masterValue.equalsIgnoreCase(targetValue);
} else {
    keysMatch   = masterKey  .equals(targetKey);
    valuesMatch = masterValue.equals(targetValue);
}

if (keysMatch) {
    if (valuesMatch) {
        doOK(masterKey, masterValue);
        break;
    } else {
        // Key is either Missing or is an Error
        if (checkErrors) {
            doError(masterKey, masterValue, targetValue);
            break;
        }
    }
}


You could make it shorter by replacing them with the booleans that they represent, ie: masterKey.equalsIgnoreCase((targetKey)) with true or false.Which could help make it shorter, because you would need one less if and else clause.


bool ignoreCase = profile.isIgnoreCase();

if ((ignoreCase && masterKey.equalsIgnoreCase(targetKey)) ||
   (!ignoreCase && masterKey.equals(targetKey))){

   if ((ignoreCase && masterValue.equalsIgnoreCase(targetValue)) || 
      (!ignoreCase && masterValue.equals(targetValue))) {

      doOK(masterKey, masterValue);
      break;
   } else
      if (checkErrors) {
         doError(masterKey, masterValue, targetValue);
         break;
      }
}

Should be quicker this way to read boolean value, if this evaluates to false the program can skip to the next evaluation sooner than running the string compare first.


don't know if you need the break there. But I believe this is completely equivalent to your code.

bool ignoreCase = profile.isIgnoreCase();

if(ignoreCase and masterKey.equalsIgnoreCase(targetKey) or !ignoreCase and masterKey.equals(targetKey)) {

    if(ignoreCase and masterValue.equalsIgnoreCase(targetValue) or !ignoreCase and masterValue.equals(targetValue)) {

        doOK(masterKey, masterValue);
        break;

    } else if(checkErrors) {

        doError(masterKey, masterValue, targetValue);
        break;
    }
}

Or I would write the following function in masterKey and masterValue class

public bool equalsCheckCase(targetKey, ignoreCase) {

    if(ignoreCase) {
        return this.equalsIgnoreCase(targetKey)
    } else {
        return this.equals(targetKey);
    }
}

So the code sample becomes more readable.

bool ignoreCase = profile.isIgnoreCase();

if(masterKey.equalsCheckCase(targetKey, ignoreCase)) {

    if(masterValue.equalsCheckCase(targetValue, ignoreCase)) {

        doOK(masterKey, masterValue);
        break;

    } else if(checkErrors) {

        doError(masterKey, masterValue, targetValue);
        break;
    }
}
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜