Исключение Redunancy в действиях ASP.NET MVC

Question

Является ли это слишком избыточным - лучше ли его упростить?

[Authorize, AcceptVerbs(HttpVerbs.Post)] 
public ActionResult ChangePassword(string oldPassword, string newPassword, string confirmPassword) 
{ 
    var oldPasswordValidationResults = _validatorProvider.Validate<IStringLengthValidator>(oldPassword); 
    oldPasswordValidationResults.Where(r => !r.Passed) 
           .Each(r => ModelState.AddModelError("OldPassword", "Please enter your old password.")); 


    var newPasswordValidationResults = _validatorProvider.Validate<IStringLengthValidator>(newPassword); 
    newPasswordValidationResults.Where(r => !r.Passed) 
           .Each(r => ModelState.AddModelError("NewPassword", "Please enter a new password.")); 

    if (!ModelState.IsValid) 
     return View(); 

    if (newPassword != confirmPassword) 
     ModelState.AddModelError("ConfirmPassword", "The passwords do not match."); 

    if (!ModelState.IsValid) 
     return View(); 

    if (!_userMembershipService.ChangePassword(oldPassword, newPassword)) 
     ModelState.AddModelError("_FORM", "Unable to change your password."); 

    if (!ModelState.IsValid) 
     return View(); 

    return View("ChangePasswordSuccessful"); 
} 

Все это, мне кажется, чтобы иметь код запах ...

if (!ModelState.IsValid) 
    return View(); 

Answer 1

Это изменение, кажется, сохранить свои первоначальные намерения немного лучше:

if (newPassword != confirmPassword) 
{ 
    ModelState.AddModelError("ConfirmPassword", "The passwords do not match."); 
    return View(); 
} 

if (!_userMembershipService.ChangePassword(oldPassword, newPassword)) 
{ 
    ModelState.AddModelError("_FORM", "Unable to change your password."); 
    return View(); 
} 

return View("ChangePasswordSuccessful"); 

Answer 2

Нет, я бы сказал, что вам нужен только последний IsValid-чек.

Конечно, вам может потребоваться сохранить второе, чтобы изменить пароль, если он будет таким же, как старый, может привести к ненужным событиям журнала или еще что-то, в зависимости от базовой структуры членства.

Answer 3

переместить все ваши основные проверки, как длина строки и типа до уровня модели, это позволит уменьшить массу кода, вы можете проверить xval рамки

Answer 4

Да, я бы просто держать последнюю IsValid чек, так:

[Authorize, AcceptVerbs(HttpVerbs.Post)] 
public ActionResult ChangePassword(string oldPassword, string newPassword, string confirmPassword) 
{ 
    var oldPasswordValidationResults = _validatorProvider.Validate<IStringLengthValidator>(oldPassword); 
    oldPasswordValidationResults.Where(r => !r.Passed).Each(r => ModelState.AddModelError("OldPassword", "Please enter your old password.")); 


    var newPasswordValidationResults = _validatorProvider.Validate<IStringLengthValidator>(newPassword); 
    newPasswordValidationResults.Where(r => !r.Passed).Each(r => ModelState.AddModelError("NewPassword", "Please enter a new password.")); 


    if (newPassword != confirmPassword) 
     ModelState.AddModelError("ConfirmPassword", "The passwords do not match."); 

    if (!_userMembershipService.ChangePassword(oldPassword, newPassword)) 
     ModelState.AddModelError("_FORM", "Unable to change your password."); 

    if (!ModelState.IsValid) 
     return View(); 

    return View("ChangePasswordSuccessful"); 
} 

Несмотря на то, что @ j-steen делает хороший вывод о проверке второго на последний день, возможно, сэкономит вам некоторые накладные расходы.

Answer 5

Вложенные if заявления может помочь упростить код:

[Authorize, AcceptVerbs(HttpVerbs.Post)] 
public ActionResult ChangePassword(string oldPassword, string newPassword, string confirmPassword) 
{ 
    var oldPasswordValidationResults = _validatorProvider.Validate<IStringLengthValidator>(oldPassword); 
    oldPasswordValidationResults.Where(r => !r.Passed) 
           .Each(r => ModelState.AddModelError("OldPassword", "Please enter your old password.")); 


    var newPasswordValidationResults = _validatorProvider.Validate<IStringLengthValidator>(newPassword); 
    newPasswordValidationResults.Where(r => !r.Passed) 
           .Each(r => ModelState.AddModelError("NewPassword", "Please enter a new password.")); 

    if (ModelState.IsValid) { 
     if (newPassword == confirmPassword) { 
      if (_userMembershipService.ChangePassword(oldPassword, newPassword)) { 
       return View("ChangePasswordSuccessful"); 
      } 
      else { 
       ModelState.AddModelError("_FORM", "Unable to change your password."); 
      } 
     } 
     else { 
      ModelState.AddModelError("ConfirmPassword", "The passwords do not match."); 
     } 
    } 

    return View(); 
}