Is there a way to shorten a conditional expression containing a bunch of logical comparisons?

Question

eg

if("viewCategoryTree".equals(actionDetail)
                || "fromCut".equals(actionDetail)
                || "fromPaste".equals(actionDetail)
                || ("viewVendorCategory".equals(actionDetail))&&"viewCategoryTree".equals(vendorCategoryListForm.getActionOrigin())
                || ("viewVendorCategory".equals(actionDetail))&&"fromEdit".equals(vendorCategoryListForm.getActionOrigin())
                || "deleteSelectedItem".equals(actionDetail)
                || ("viewVendorCategory".equals(actionDetail))&&"fromLink".equals(vendorCategoryListForm.getActionOrigin())){
//do smth
}

I tried something like this

if(check("deleteSelectedItem,viewCategoryTree,fromCut,fromPaste,{viewVendorCategory&&viewVendorCategory},{viewVendorCategory&&fromEdit},{viewVendorCategory&&fromLink}",actionDetail,actionOrigin)){
//do smth
}

public boolean check(String str, String ad, String ao){

    String oneCmp = "";
    String[] result = str.split(",");
    ArrayList adList = new ArrayList();
    ArrayList aoList = new ArrayList();
    for (int i=0; i<result.length; i++){
        oneCmp = result[i];
        Matcher m = Pattern.compile("\\{([^}]*)\\}").matcher(oneCmp);
        if(m.matches()){
            m.find();
            String agrp = m.group();
            String[] groupresult = agrp.split("[\\W&&[^!]]+");
            Boolean a = false;
            Boolean b = false;
            if(groupresult[0].startsWith("!")){
                a = !groupresult[0].substring(1).equals(ad);
            } else a = groupresult[0].equals(ad);
            if(groupresult[1].startsWith("!")){
                b = !groupresult[1].substring(1).equals(ao);
            }else b = groupresult[1].equals(ao);

            if(agrp.indexOf("&&")!=-1){
                if(!(a && b))return false;
            }
            else if(agrp.indexOf("||")!=-1){
                if(!(a || b))return false;
            }
        } else {
            if(oneCmp.indexOf("^")==-1){
                checklist(oneCmp,ad);
                        if(!checklist(oneCmp,ad))return false;
            }else{
            if(!checklist(oneCmp,ao))return false;
            }
        }
    }

    return false;
}

public boolean checklist(String str, String key){

    if(str.startsWith("!")){
        if(str.substring(1).equals(key))return false;
        }else { if (!str.substring(1).equals(key)) return false;
        }
    }

    return false;
}

Is there a better way to do this? thank.

Answer 1

Move the check to a method that takes actionDetailas an argument:

// Assumes vendorCategoryListForm is a member variable.
boolean check(String actionDetail) {
    return ("viewCategoryTree".equals(actionDetail)
            || "fromCut".equals(actionDetail)
            || "fromPaste".equals(actionDetail)
            || (("viewVendorCategory".equals(actionDetail))
                &&"viewCategoryTree".equals(vendorCategoryListForm.getActionOrigin()))
            || (("viewVendorCategory".equals(actionDetail))
                &&"fromEdit".equals(vendorCategoryListForm.getActionOrigin()))
            || "deleteSelectedItem".equals(actionDetail)
            || (("viewVendorCategory".equals(actionDetail))
                &&"fromLink".equals(vendorCategoryListForm.getActionOrigin())))
}

if (check(actionDetail)) {
    // do this
}

Answer 2

How to create an array of what you need to check. And then some code looks like this:

arrayOfStrings = ["viewCategoryTree", ...]
match = false
for elem in arrayOfStrings:
   if elem == actionDetail:
       match = true
       break

The good thing about an array is that it expands easily: you can easily add / remove elements, both statically and dynamically.

Answer 3

.

   1. Replace conditions with guard clauses.
   2. Decompose conditional blocks into seperate functions.
   3. Convert negative checks into positive checks.

Answer 4

, . - if (control.IsApplicable) { // do smth }.

, . , , , , , true.

Answer 5

, , , , , "", .

The problem with writing is that: while you can express conditions in fewer characters, someone else reading your code should figure out what this funky string literal actually means.

In addition, any smart you can affect performance. For example, your attempt to compile and apply a regex several times for each call check.

Stick to what you have will be my advice.

Answer 6

if(isValidActionDetail(actionDetail)
            || (isValidActionDetail(actionDetail)
            && ("viewCategoryTree".equals(vendorCategoryListForm.getActionOrigin()) 
                || "fromEdit".equals(vendorCategoryListForm.getActionOrigin())  
                || "fromLink".equals(vendorCategoryListForm.getActionOrigin())))){

//do smth
    }
}

public static boolean isValidActionDetail (String actionDetail) {
    return "viewCategoryTree".equals(actionDetail) || "fromCut".equals(actionDetail) 
           || "fromPaste".equals(actionDetail) || "deleteSelectedItem".equals(actionDetail) 
           || "viewVendorCategory".equals(actionDetail);
}

You can decompose this above as a first step to refactoring your logic.