MVC 控制器中的单一职责原则。需要批评答案

【问题标题】：Single Responsibility Principle in MVC controllers. Critique requiredMVC 控制器中的单一职责原则。需要批评
【发布时间】：2012-12-05 00:11:53
【问题描述】：

在我的 MVC4 应用程序中，有一些操作需要根据您是否登录（在我的情况下为 FormsAuthentication）而采取不同的行为。

例如，我有一个 AccountController，它有一个方法“RenderAccountAndProfile”。如果注销，相应的局部视图会显示登录提示和按钮。如果用户已登录，则会显示用户的个人资料链接以及注销按钮。

到目前为止，我在项目中采用的方法是简单地使用 if 语句...

        if (HttpContext.User.Identity.IsAuthenticated)
        {
            ...
        }
        else
        {
            ...
        }

但是，我刚刚创建了一个我认为是这种方法的相当优雅的替代方案。

我创建了一个名为 AnonymousUsersOnly 的新属性，非常简单：

public class AnonymousUsersOnlyAttribute : System.Web.Mvc.ActionMethodSelectorAttribute
{
    public override bool IsValidForRequest(System.Web.Mvc.ControllerContext controllerContext, System.Reflection.MethodInfo methodInfo)
    {
        return !controllerContext.HttpContext.User.Identity.IsAuthenticated;
    }
}

我的 AccountController 类装饰有 Authorize 属性。这使我能够拥有以下代码：

[Authorize]
public class AccountController : Controller
{
    [AllowAnonymous]
    [AnonymousUsersOnly]
    [ActionName("RenderAccountAndProfile")]
    public ActionResult RenderAccountAndProfile_Anonymous()
    {
        // create a "logged out" view model
        return Content("**NOT LOGGED IN** - LOG IN HERE");
    }

    [ActionName("RenderAccountAndProfile")]
    public ActionResult RenderAccountAndProfile_Authorized()
    {
        // create a "logged in" view model
        return Content("**LOGGED IN** - LOG OUT");
    }
}

我非常喜欢这种方法，因为我的操作方法符合Single Responsibility Principle。每种方法现在只处理登录情况或注销情况。我不再需要任何“if”语句来引导流量。

这也应该使单元测试更容易，因为每个方法现在只关注一个结果，而不是两个。我们可以编写单元测试来分别测试每个结果，调用不同的方法。

显然，我不能有两个具有相同签名的方法，因此我必须使用 ActionName 属性。

我会很感激你在这里的批评。你认为这是一个优雅的解决方案吗？这种方法的优点和缺点是什么？这会带来哪些安全隐患/风险？

【问题讨论】：

为什么不直接在action级别使用Authorize属性，让mvc在调用action时检查访问权限，自动带你到登录控制器？
这与我想要的功能不太一样。我不希望发生重定向。我在上面创建的是一个过滤器，用于根据用户是否经过身份验证来选择要调用的操作方法。此外，使用 [Authorize] 装饰整个类并使用 [AllowAnonymous] 在其中打孔是一种更安全的方法，因为您不能忘记对所有公共方法应用授权。否则，一个错误就会导致您的整个应用受到威胁。
这个的典型用例是装饰呈现部分视图的操作方法，而不是 ViewResults，尽管它当然不限于此。我个人打算使用这种方法来根据用户是否被授权显示不同的内容。

标签： asp.net-mvc asp.net-mvc-3 design-patterns attributes single-responsibility-principle

【解决方案1】：

这里的问题是策略模式问题。而且您已经实现了一个（非标准）策略模式，并且实现得非常巧妙。我担心它太聪明了。这种聪明才智使得代码所做的事情对于外行来说不那么明显。

顺便说一句，我宁愿不打扰。我经常将控制器编写为域对象/服务上的非常薄的适配器。因此，我愿意采取务实的态度来完善控制器。在轻微的设计问题和明显的代码之间做出决定时，总是选择明显的代码。

如果您有更厚的控制器，或者出于其他原因需要在这里真正关注此问题，您可能会考虑使用更传统的策略模式，也许可以借助抽象工厂来提供基于身份验证状态的不同策略实现。这符合您的设计目标，并且其他程序员会更加熟悉（如果他们知道设计模式）。

话虽如此，我认为保留您的聪明解决方案不会对任何事情造成太大伤害。我很想改名；拒绝对我来说似乎是一个奇怪的动词。或许AnonymousUsersOnly，这对未来的程序员来说会更有交流性。

【讨论】：

我完全同意你的观点，务实的方法往往比任何聪明的方法都要好。但我的解决方案真的那么聪明吗？它本质上是 1 行代码。我个人认为它很优雅，但很聪明，不。如果我们担心外行无法理解正在发生的事情，那么为什么整个 MVC 框架首先允许我们对过滤器进行子类化呢？对于任何想成为 MVC 程序员的人来说，过滤器是一种必不可少的学习。顺便说一下，我真的很喜欢你关于命名约定的建议。我将编辑我的原始帖子以反映这一点。
继我之前的评论之后（由于缺少字符！），几乎每本 MVC 书籍或培训指南都有一整章专门用于创建自定义过滤器。只要存在命名约定（例如 RenderAccountAndProfile_Anonymous 或 RenderAccountAndProfile_Authorized），那么一目了然就应该很容易阅读发生的事情，不是吗？
当然。我不认为这是一个糟糕的解决方案。风险在于未来的维护者不理解。他们可能不明白您的名字是否对他们没有意义（我的名字一直对我有意义，并让其他人感到困惑），或者您的方法命名约定是否未遵循，或者这些方法是否在文件中在空间上分开.如果您遇到的情况只有一个或另一个，而不是两者都有，这也可能是一个问题。根据您的上下文，所有这些风险可能都很低（例如，您有一个 MVC 忍者团队在同一地点）。如果您觉得这些风险很低，那就去做吧。
tallseth，感谢您的 cmets，我发现它非常有用。对我来说，它只是“感觉不错”。我花了几个小时使用这种方法重新设计了一个控制器，我对合适的方法看起来和感觉更干净，现在执行一个单一的功能感到非常满意。但是，我已经认真对待你们的 cmets，我的方法有时间和地点，它必须在正确的环境中。

【解决方案2】：

我最好选择（伪代码）：

PartialView UserProfile() { ... }

PartialView Login() { ... }

并且在视图中：

if (User.IsAuthenticated) {
    @Html.Action("UserProfile")
} else {
    @Html.Action("Login")
}

它也可以是 DisplayTemplate、助手或任何你喜欢的东西，所以你最终会使用

@Html.DisplayFor(m=> User)

【讨论】：