Maintainable methods



  • Which do you think is a better method of forming a boolean test:
    A:
        boolean someTest(){
            boolean retVal = false;
            if(someCondition){
                retVal = true;
            }
            return retVal;
        }
    B:
        boolean someTest(){
            if(someCondition){
                return true;
            }
            return false;
        }
    C:
        boolean someTest(){
            return someCondition;
        }

    Where someCondition may be a complex statement.



  •  FAIL.

    I see no provision for FILE_NOT_FOUND.

    Noob.


  • ♿ (Parody)

    @MasterPlanSoftware said:

    I see no provision for FILE_NOT_FOUND.
    Well, you might get that from option C, for certain definitions of boolean.  At least, the kind of definitions you tend to get on this site.



  • C



  • I guess I've used all three forms, and I would not spend too much time thinking about which one is best, when all you really need is a dice. 



  • @cBradley said:

    Which do you think is a better method of forming a boolean test:
    A:
        boolean someTest(){
            boolean retVal = false;
            if(someCondition){
                retVal = true;
            }
            return retVal;
        }
    B:
        boolean someTest(){
            if(someCondition){
                return true;
            }
            return false;
        }
    C:
        boolean someTest(){
            return someCondition;
        }

    Where someCondition may be a complex statement.

    Imho none of them are a good method when you expect someCondition to be a reasonably complex statement. Complex guard expressions that run for several lines in cases A and B do not make for readable code. Neither does a complex expression all crammed onto a single return in case C.

    A better idea would be to split the evaluation of someCondition into smaller parts, incrementally dismissing cases on a logical basis. Meaning:

    boolean someTest(){
    // Explain part of condition
    if (!somePartOfCondition)
    return false;

    // Explain other part of condition
    if (!someOtherPartOfCondition)
    return false;

    // Explain what cases remain
    return true;
    }

    I guess that matches closest to type B.



  •  A or B or C.

    It depends on the complexity of someStatement and the surrounding code.

    Also, I despise the varname retVal. It means nothing. 



  • In order of preference, C, B, A.

    I hate pointless retVals. I once had to maintain a module absolutely littered with them. Every single place where a pointless retVal could possibly be implemented had one.



  • Most of the time, the function will be much longer, so I choose A. This gives you more debugging capabilities to test multiple conditions so you can verify it after every step.  If it is super simple, and a single condition, I would use C. I usually shy away from method B, method A is better because it has 1 return call. putting the return in the if would probably not buy you any major performance benefit, and our company's coding standard uses method A, so thats why I'm kinda biased.



  • @Ragnax said:

    Imho none of them are a good method when you expect someCondition to be a reasonably complex statement. Complex guard expressions that run for several lines in cases A and B do not make for readable code. Neither does a complex expression all crammed onto a single return in case C.

    A better idea would be to split the evaluation of someCondition into smaller parts, incrementally dismissing cases on a logical basis. Meaning:

    boolean someTest(){
    // Explain part of condition
    if (!somePartOfCondition)
    return false;

    // Explain other part of condition
    if (!someOtherPartOfCondition)
    return false;

    // Explain what cases remain
    return true;
    }

    I guess that matches closest to type B.

    Seconded.  C is good for simple cases but I prefer to avoid "arrow code" and do immediate returns for more complex evaluations. 


Log in to reply