CODEWITHSUNDEEP
javaif-statementrefactoring
asha n
asha n

Reputation: 33

To refactor multiple || and && conditions inside If statement in java

To refactor multiple || and && conditions inside If statement in java

To refactor multiple || and && conditions inside If statement in java

To refactor multiple || and && conditions inside If statement in java

for (Student students : studentList) {

    if ((Constants.CODE1.equals(students.getactivities().getCode())
            && ValidationRepository.validateStudentId1(department.getId()))
            || ((Constants.CODE2).equals(students.getactivities().getCode())
                    && ValidationRepository.validateStudentId2(department.getId())
                    && ValidationRepository.validateStudentId3(department.getId()))
            || ((Constants.CODE3
                    .equals(students.getactivities().getCode())
                    || Constants.CODE4
                            .equals(students.getactivities().getCode()))
                    && (ValidationRepository.validateStudentId4(
                            department.getId(),
                            students.getactivities().getCode())))
            || ((Constants.CODE5.equals(students.getactivities().getCode())
                    || Constants.CODE6
                            .equals(students.getactivities().getCode())

                    || Constants.CODE7
                            .equals(students.getactivities().getCode())
                    || Constants.CODE15
                            .equals(students.getactivities().getCode())

                    || Constants.CODE8
                            .equals(students.getactivities().getCode())
                    || Constants.CODE9
                            .equals(students.getactivities().getCode())

                    || Constants.CODE10
                            .equals(students.getactivities().getCode())
                    || Constants.CODE11
                            .equals(students.getactivities().getCode()))

                    && (ValidationRepository.validateStudentId4(
                            department.getId(),
                            students.getactivities().getCode())))

    )
    { 
        some statements
    }

Upvotes: 1

Views: 1442

Answers (3)

kiranNswamy
kiranNswamy

Reputation: 126

Create a method to validate student Inside the method create a map of Constant code and validation repo function

public boolean validateStudent(String constantCode, departmentId){
Map<String, Boolean> map = new HashMap<>();
map.put(Constants.CODE1,  
ValidationRepository.validateStudentId1(departmentId));
map.put(Constants.CODE2,  
ValidationRepository.validateStudentId2(departmentId));

if(map.containsKey(constantCode)){
        return map.get(operation);
    
    }
    
    return false;


 }

Now you can call validate method like below

validateStudent(constantCode, departmentId)

Internally the map value for the constant code key will be your function. Make sure you use the same parameter name of method and key value in map.

Upvotes: 1

Michal Korzeniewski
Michal Korzeniewski

Reputation: 1

Let me give you a piece of advice:

  1. I believe you can consist all the codes in the list and check weather students.getactivities().getCode() is present in the list.
listOf(Constants. CODE7, Constants.CODE8, Constants.CODE9 ...).contains(students.getactivities().getCode());
  1. You should also extract a block of the code to the external methods:
private validateStudentWithCode1(Students students) 
{
return Constants.CODE1.equals(students.getactivities().getCode())
                         && ValidationRepository.validateStudentId1(department.getId());
}

Upvotes: 0

Jolly
Jolly

Reputation: 209

This question has no clear answer, but I have the following suggestions:

  1. Do not call students.getactivities().getCode() every single time. Not only does it make the code less readable, it is also ineffective. (Unless this method does not return the same value every time!) Rather use a variable before the if-statement:
int code = students.getactivities().getCode();

Then, you can just refer to that variable at any time. The same can be done for department.getId().

  1. You can use a helper method to avoid having to compare that code to multiple constants, like this:
private boolean matchesAny(int code, int... possibleMatches) {
    for (int match: possibleMatches) {
        if (code == match) return true;
    }
    return false;
}

That way, your code will look like this:

    int departmentId = department.getId();
    for (Student students : studentList) {
        int code = students.getactivities().getCode();
        // Case 1
        if ((Constants.CODE1.equals(code)
                && ValidationRepository.validateStudentId1(departmentId))
                // Case 2
                || (Constants.CODE2.equals(code)
                && ValidationRepository.validateStudentId2(departmentId)
                && ValidationRepository.validateStudentId3(departmentId))
                // Case 3
                || (matchesAny(code, Constants.CODE3, Constants.CODE4)
                && ValidationRepository.validateStudentId4(departmentId, code))
                // Case 4
                || (matchesAny(code,
                    Constants.CODE5,
                    Constants.CODE6,
                    Constants.CODE7,
                    Constants.CODE8,
                    Constants.CODE9,
                    Constants.CODE10,
                    Constants.CODE11,
                    Constants.CODE15)
                && ValidationRepository.validateStudentId4(departmentId, code))) {
            // some statements
        }
    }
    // rest of method

private boolean matchesAny(int code, int... possibleMatches) {
    for (int match : possibleMatches) {
        if (code == match) return true;
    }
    return false;
}

Still not beautiful, but already a lot better. To make it even better, you might try and turn the 4 cases into separate statements and have them all call the same code afterwards.

Upvotes: 0

Related Questions