How to tidy up too many if statements for readability

Question

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?

Answer 1

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

Answer 2

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.
}

Answer 3

Better to split it into multiple methods(functions) .It will be easy to understand.If some new person reads the code he/she understands the logic just by reading the method name itself(Note: Method name should express what test it does).Sample code :

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

if(isValidParameter(userId)){
  User user=new User(userId);
    if(isValidUser(user)&&isUserHasAccess(user)){
      //Do whatever you want
     }
}

private boolean isUserHasAccess(User user){
    if (user.hasAccess){
       return true;
    }else{
        denyAccess(INVALID_ACCESS);
       return false;
    }
}

 private boolean isValidUser(User user){
    if(user !=null){
      return true;
    }else{
    denyAccess(INVALID_USER);
    return false;
    }
 }


 private boolean isValidParameter(String userId){
    if(userid !=""){
      return true;
    }else{
 denyAccess(INVALID_PARAMETER);
   return false;
  }
}