Skip to main content
added 1346 characters in body
Source Link

Supposing all functions are implemented in the same component, you could make the functions retain some state in order to get rid of the multiple branches in the flow.

EG: checkCondition1() would become evaluateCondition1(), on which it would check if previous condition 1 iswere met; isif so, then it savescaches some value to be retrieved by getConditionNumber().

checkCondition2() would become evaluateCondition2(), on which it would check if the previous condition 1 wasconditions were met. If previous condition was not met, then it checks for condition scenario 2, savingcaching a cache value to be retrieved by getConditionNumber(). And so on.

clearConditions();
evaluateCondition1();
evaluateCondition2();
evaluateCondition3();
evaluateCondition4();
if (anyCondition()) { addAlert(getConditionNumber()); }

EDIT:

Here's how the check for expensive conditions would need to be implemented in order to this approach work.

bool evaluateCondition34() {
    if (!anyCondition() && A && B && C) {
        conditionNumber = 5693;
        return true;
    }
    return false;
}

...

bool evaluateCondition76() {
    if (!anyCondition() && !B && C && D) {
        conditionNumber = 7658;
        return true;
    }
    return false;
}

Therefore, if you have too many expensive checks to be performed, and things in this code remains private, this approach helps maintaining it, enabling to change the order of the checks if necessary.

clearConditions();
evaluateCondition10();
evaluateCondition9();
evaluateCondition8();
evaluateCondition7();
...
evaluateCondition34();
...
evaluateCondition76(); 

if (anyCondition()) { addAlert(getConditionNumber()); }

This answer just provides some alternative suggestion from the other answers, and probably will not be better than the original code if we consider only 4 lines of code. Although, this is not a terrible approach (and neither makes maintenance more difficult like others have said) given the scenario I mentioned (too many checks, only main function exposed as public, all functions are implementation details of the same class).

Supposing all functions are implemented in the same component, you could make the functions retain some state in order to get rid of the multiple branches in the flow.

EG: checkCondition1() would become evaluateCondition1(), on which it would check condition 1 is met; is so then it saves some value to be retrieved by getConditionNumber().

checkCondition2() would become evaluateCondition2(), on which it would check if the previous condition 1 was met. If previous condition was not met, then it checks for condition scenario 2, saving a cache value to be retrieved by getConditionNumber(). And so on.

evaluateCondition1();
evaluateCondition2();
evaluateCondition3();
evaluateCondition4();
if (anyCondition()) { addAlert(getConditionNumber()); }

Supposing all functions are implemented in the same component, you could make the functions retain some state in order to get rid of the multiple branches in the flow.

EG: checkCondition1() would become evaluateCondition1(), on which it would check if previous condition were met; if so, then it caches some value to be retrieved by getConditionNumber().

checkCondition2() would become evaluateCondition2(), on which it would check if the previous conditions were met. If previous condition was not met, then it checks for condition scenario 2, caching a value to be retrieved by getConditionNumber(). And so on.

clearConditions();
evaluateCondition1();
evaluateCondition2();
evaluateCondition3();
evaluateCondition4();
if (anyCondition()) { addAlert(getConditionNumber()); }

EDIT:

Here's how the check for expensive conditions would need to be implemented in order to this approach work.

bool evaluateCondition34() {
    if (!anyCondition() && A && B && C) {
        conditionNumber = 5693;
        return true;
    }
    return false;
}

...

bool evaluateCondition76() {
    if (!anyCondition() && !B && C && D) {
        conditionNumber = 7658;
        return true;
    }
    return false;
}

Therefore, if you have too many expensive checks to be performed, and things in this code remains private, this approach helps maintaining it, enabling to change the order of the checks if necessary.

clearConditions();
evaluateCondition10();
evaluateCondition9();
evaluateCondition8();
evaluateCondition7();
...
evaluateCondition34();
...
evaluateCondition76(); 

if (anyCondition()) { addAlert(getConditionNumber()); }

This answer just provides some alternative suggestion from the other answers, and probably will not be better than the original code if we consider only 4 lines of code. Although, this is not a terrible approach (and neither makes maintenance more difficult like others have said) given the scenario I mentioned (too many checks, only main function exposed as public, all functions are implementation details of the same class).

Source Link

Supposing all functions are implemented in the same component, you could make the functions retain some state in order to get rid of the multiple branches in the flow.

EG: checkCondition1() would become evaluateCondition1(), on which it would check condition 1 is met; is so then it saves some value to be retrieved by getConditionNumber().

checkCondition2() would become evaluateCondition2(), on which it would check if the previous condition 1 was met. If previous condition was not met, then it checks for condition scenario 2, saving a cache value to be retrieved by getConditionNumber(). And so on.

evaluateCondition1();
evaluateCondition2();
evaluateCondition3();
evaluateCondition4();
if (anyCondition()) { addAlert(getConditionNumber()); }