How to remove too much if statements for readability

Question

I have an example of some code that I often see on websites that I would like to improve, and would appreciate some help. Often I see 5-10 nested if statements in the page_load method, which are aimed at eliminating invalid user input, but it looks ugly, it is difficult to read and maintain.

How do you recommend clearing the following code sample? 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, it gets very dirty very fast! Are there any patterns or methods that I must follow in this case?

Answer 1

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 error

Answer 2

You can clear the nesting a bit by canceling the conditions and write down the 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

It is better to break it into several methods (functions). It will be easy to understand. If some new person reads the code, he / she understands the logic by simply reading the name of the method (Note: the name of the method should express which test it does). Code example:

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