Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to tidy up too many if statements for readability

I have an example of some code that I see often in websites that I'd like to improve and would appreciate some help. Often I see 5-10 nested if-statements in a page_load method which aim to eliminate invalid user input, but this looks ugly and is hard to read and maintain.

How would you recommend cleaning up the following code example? The main thing I'm trying to eliminate is the nested if statements.

string userid = Request.QueryString["userid"];

if (userid != ""){
    user = new user(userid);

    if (user != null){
        if (user.hasAccess){
            //etc.
        }
        else{
            denyAccess(INVALID_ACCESS);
        }
    }
    else{
        denyAccess(INVALID_USER);
    }
}
else{
    denyAccess(INVALID_PARAMETER);
}

As you can see, this gets quite messy very quickly! Are there any patterns or practices that I should be following in this case?

like image 204
NickGPS Avatar asked Oct 30 '09 11:10

NickGPS


People also ask

How do I clean my nested IF statement?

To eliminate a nested conditional statement, you can use a guard clause. A guard clause is a condition within the if statement that must be met for code execution to continue. If the condition isn't met, then no further processing is done. The guard clause favors an early return from the method.


2 Answers

By using Guard Clauses sir

string userid = Reuest.QueryString["userid"];

if(userid==null)
 return denyAccess(INVALID_PARAMETER);

user = new user(userid);
if(user==null)
 return denyAccess(INVALID_USER);

if (!user.hasAccess)
 return denyAccess(INVALID_ACCESS);

//do stuff

PS. use either return or throw an error

like image 90
lemon Avatar answered Oct 26 '22 19:10

lemon


You can clean up the nesting a bit by negating the conditions and write an if-else chain:

string userid = Reuest.QueryString["userid"];

if (userid == "") {
    denyAccess(INVALID_PARAMETER);

} else if (null == (user = new user(userid))){
    denyAccess(INVALID_USER);

} else if (!user.hasAccess){
    denyAccess(INVALID_ACCESS);

} else {
    //etc.
}
like image 35
rsp Avatar answered Oct 26 '22 19:10

rsp