Làm cách nào để dọn dẹp quá nhiều câu lệnh nếu có thể đọc được

Question

Tôi có một ví dụ về một số mã mà tôi thường thấy trong các trang web mà tôi muốn cải thiện và sẽ đánh giá cao một số trợ giúp. Thường thì tôi thấy 5-10 câu lệnh if lồng nhau trong một phương thức page_load nhằm loại bỏ đầu vào người dùng không hợp lệ, nhưng điều này có vẻ xấu xí và khó đọc và duy trì.

Bạn sẽ đề xuất làm sạch ví dụ mã sau như thế nào? Điều chính tôi đang cố gắng loại bỏ là các câu lệnh lồng nhau nếu.

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

Như bạn thấy, điều này khá lộn xộn rất nhanh! Có bất kỳ mẫu hoặc thực tiễn nào mà tôi nên tuân theo trong trường hợp này không?

Answer 1

Bằng cách sử dụng Guard Clauses thưa ông

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. sử dụng hoặc trả lại hoặc ném ra một lỗi

Answer 2

Bạn có thể dọn dẹp làm tổ một chút bằng cách phủ nhận điều kiện và viết một if-else chuỗi:

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

Tốt hơn để chia nó thành nhiều phương pháp (chức năng) Nó sẽ dễ hiểu. Nếu một số người mới đọc mã, anh/cô ấy hiểu được logic chỉ bằng cách đọc tên phương thức đó (Ghi chú: Tên phương pháp cần thể hiện thử nghiệm của nó). Mã mẫu:

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