c# - 如何从 MVC Controller 中提取错误处理代码？

Question

大多数时候，我都能想出如何将业务逻辑从 MVC Controller 分解为服务或模型。然而，错误处理是个异常(exception)，在我看来这是 Controller 的责任。然而，它可能导致相当“不瘦”的 Controller 。例如:

if ((foundEvent = repoEvent.GetEventById(id)) == null) {
    return HttpNotFound("Could not find event with id {0}.".FormatWith(id));
}

var assessment = isSample ? foundEvent.SampleAssessment : foundEvent.ActualAssessment;
if (assessment == null) {
    return HttpNotFound("Could not find {0}assessment for event with id {1}".FormatWith(isSample ? "sample " : "", id));
}

if (assessment.UnmappedCaseStudiesCount == 0) {
    TempData[TempDataKeys.ErrorMessage.GetEnumName()] = "This assessment (\"{0}\") has no case studies!".FormatWith(assessment.Name);
    return RedirectToAction("Index", "Events");
}

在上面的所有情况下，在我看来，逻辑确实属于 Controller ，因为 Controller 正在设置错误消息而不是返回 View (这被 HttpNotFound 这样的事实所强化 和 RedirectToAction 是 Controller 类的成员，因此对于未扩展 Controller 的类不可用。然而，您可以看到这会在不久后变得又长又乱。有没有办法在 Controller 中排除这种错误处理，使 Controller 更“瘦”，或者这些东西只是属于 Controller ？

这是一个更大的代码片段，说明我如何重构以允许 2 个操作方法使用相同的 View 模型设置代码。但是，它仍然会在 Controller 中产生 90 多行代码，仅用于 2 个操作方法；再次，不是一个非常“瘦”的 Controller :

#region Private methods
private StartAssessmentViewModel getStartViewModel(int eventId, bool isSample, out ActionResult actionRes) {
    actionRes = null;

    EventRepository repoEvent = new EventRepository();
    Event foundEvent;
    if ((foundEvent = repoEvent.GetEventById(eventId)) == null) {
        actionRes = HttpNotFound("Could not find event with id {0}.".FormatWith(eventId));
        return null;
    }

    var assessment = isSample ? foundEvent.SampleAssessment : foundEvent.ActualAssessment;
    if (assessment == null) {
        actionRes = HttpNotFound("Could not find {0}assessment for event with id {1}".FormatWith(isSample ? "sample " : "", eventId));
        return null;
    }

    if (assessment.UnmappedCaseStudiesCount == 0) {
        TempData[TempDataKeys.ErrorMessage.GetEnumName()] = "This assessment (\"{0}\") has no case studies!".FormatWith(assessment.Name);
        actionRes = RedirectToAction("Index", "Events");
        return null;
    }

    try {
        // Has the assessment finished? (samples don't count)

        UserAssessmentRepository repoUa = new UserAssessmentRepository();
        var foundUa = repoUa.GetUserAssessment(foundEvent.EventId, assessment.AssessmentId, User.Identity.Name);
        // TODO: check that foundUa.Assessment.IsSample is OK; may need to make .Assessment a concrete instance in the repo method
        if (foundUa != null && !foundUa.Assessment.IsSample) {
            if (_svcAsmt.IsAssessmentFinished(foundUa)) {
                // TODO: test that this way of displaying the error works.
                TempData[TempDataKeys.ErrorMessage.GetEnumName()] = "You have already completed the assessment for this event ('{0}'); you cannot start it again.".FormatWith(foundEvent.Name);
                actionRes = RedirectToAction("Index", "Events");
                return null;
            }

            // Has it been started already?
            if (_svcAsmt.IsAssessmentStarted(foundEvent.EventId, foundUa.AssessmentId, User.Identity.Name)) {
                actionRes = RedirectToAction("Question", new { id = foundUa.UserAssessmentId });
                return null;
            }
        }

        return Mapper.Map<StartAssessmentViewModel>(assessment);
    }
    catch (Exception ex) {
        TempData[TempDataKeys.ErrorMessage.GetEnumName()] = "Could not display start screen for assessment {0} on event {1}; error: {2}".FormatWith(assessment.AssessmentId, foundEvent.EventId, ex.Message);
        actionRes = RedirectToAction("Index", "Events");
        return null;
    }
}
#endregion

public ActionResult Start(int id, bool isSample = false) {
    // Set up view model
    ActionResult actionRes;
    StartAssessmentViewModel viewModel = getStartViewModel(id, isSample, out actionRes);
    if (viewModel == null) {
        return actionRes;
    }

    return View(viewModel);
}

[HttpPost, ActionName("Start")]
public ActionResult StartConfirmed(int id, StartAssessmentViewModel viewModel) {
    // Set up view model
    ActionResult actionRes;
    StartAssessmentViewModel newViewModel = getStartViewModel(id, viewModel.AssessmentIsSample, out actionRes);
    if (newViewModel == null) {
        return actionRes;
    }

    if (!ModelState.IsValid) {
        return View(newViewModel);
    }

    // Model is valid; if it's not a sample, we need to check the access code
    if (!viewModel.AssessmentIsSample) {
        if (viewModel.AccessCode != "12345") {
            // Invalid access code
            TempData[TempDataKeys.ErrorMessage.GetEnumName()] = "The access code '{0}' is incorrect.".FormatWith(viewModel.AccessCode);
            return View(newViewModel);
        }
    }

    // Access code is valid or assessment is sample; redirect to first question
    return RedirectToAction("Question", new { id = 1 });
}

Answer 1

我同意这种错误处理是 Controller 的职责，所以我认为您做对了。关于使方法“更瘦”，您可能比查看 Bob Martin 的 Clean Code 指南做得更糟，该指南建议将较大的方法重构为更小的方法，如下所示:

if (assessment.UnmappedCaseStudiesCount == 0) {
    TempData[TempDataKeys.ErrorMessage.GetEnumName()] = 
        "This assessment (\"{0}\") has no case studies!".FormatWith(assessment.Name);
    actionRes = RedirectToAction("Index", "Events");
    return null;
}

...使用辅助方法，像这样:

if (AssessmentHasNoCaseStudies(assessment, out actionRes)) {
    return null;
}

...

private bool AssessmentHasNoCaseStudies(Assessment assessment, out ActionResult actionRes)
{
    actionRes = (assessment.UnmappedCaseStudiesCount == 0)
        ? RedirectToAction("Index", "Events")
        : null;

    if (actionRes == null)
    {
        return false;
    }

    TempData[TempDataKeys.ErrorMessage.GetEnumName()] = 
        "This assessment (\"{0}\") has no case studies!".FormatWith(assessment.Name);

    return true;
}