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?
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.
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
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.
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With