2

大多数时候，我可以弄清楚如何将 MVC 控制器中的业务逻辑分解为服务或模型。然而，这个例外是错误处理，在我看来这是控制器的责任。但是，它可能会导致相当“瘦”的控制器。例如：

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

HttpNotFound在上述所有情况下，在我看来，逻辑确实属于控制器，因为控制器正在设置错误消息而不是返回视图（这是由于类似和RedirectToAction是类成员的事实而得到加强的Controller，因此对于不扩展的类不可用Controller）。但是，您可以看到这会在短时间内变得漫长而混乱。有没有办法在控制器中排除这种错误处理以使控制器更“瘦”，或者这些东西只是属于控制器？

这是一个更大的代码片段，说明我如何重构以允许 2 个操作方法使用相同的视图模型设置代码。但是，它仍然会导致控制器中仅针对 2 个操作方法的 90 多行代码；再次，不是一个非常“瘦”的控制器：

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

1 回答 1

1

我同意这种错误处理是控制器的责任，所以我认为你已经把它放在了正确的地方。关于使方法“更精简”，您可能比查看 Bob Martin 的清洁代码指南做得更糟，该指南建议将较大的方法重构为更小的方法，如下所示：

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;
}
于 2012-10-23T12:35:13.243 回答