15

我刚刚接手了一个 ASP.NET MVC 项目,需要进行一些重构,但我想获得一些关于最佳实践的想法/建议。

该站点有一个 SQL Server 后端,这里是对解决方案中项目的回顾:

  • DomainObjects(每个数据库表一个类)
  • DomainORM(将代码从对象映射到数据库)
  • 模型(业务逻辑)
  • MVC(常规 ASP.NET MVC Web 设置)----控制器 ---- ViewModels ---- Views ---- Scripts

我看到的第一个“问题”是,虽然域对象类几乎是POCO,在计算字段周围有一些额外的“获取”属性,但域对象中有一些表示代码。例如,在 DomainObjects 项目中,有一个Person对象,我在该类上看到了这个属性:

 public class Person
 {

    public virtual string NameIdHTML
    {
        get
        {
           return "<a href='/People/Detail/" + Id + "'>" + Name + "</a> (" + Id + ")";
        }
    }
 }

所以显然在域对象中包含 HTML 生成的内容似乎是错误的。

重构方法

  1. 我的第一直觉是将其移至 MVC 项目中的 ViewModel 类,但我看到有很多视图会命中此代码,因此我不想在每个视图模型中重复代码。

  2. 第二个想法是创建 PersonH​​TML 类,它可以是:

    2a。在构造函数中接收 Person 的包装器或

    2b。从 Person 继承的类,然后具有所有这些 HTML 呈现方法。

    视图模型会将任何 Person 对象转换为 PersonH​​TML 对象并将其用于所有呈现代码。

我只是想看看:

  1. 如果这里有最佳实践,因为这似乎是一个常见的问题/模式

  2. 考虑到这种当前状态有多糟糕,因为除了感觉错误之外,它并没有真正导致理解代码或创建任何不良依赖关系的任何重大问题。任何有助于描述为什么让代码处于这种状态的论据从真正的实际意义上是不好的(相对于理论上的关注点分离论点)都会有所帮助,并且团队中存在是否值得改变的争论。

4

7 回答 7

11

我喜欢待定的评论。这是错误的,因为您将域关注点与 UI 关注点混合在一起。这会导致您可以避免的耦合。

至于您建议的解决方案,我真的不喜欢其中任何一个。

  1. 引入视图模型。是的,我们应该使用视图模型,但我们不想让 HTML 代码污染它们。因此,如果您有一个父对象、人员类型,并且您想在屏幕上显示人员类型,则使用视图的示例。您将使用人员类型名称而不是完整的人员类型对象填充视图模型,因为您只需要屏幕上的人员类型名称。或者,如果您的域模型的名字和姓氏是分开的,但您的视图调用 FullName,您将填充视图模型的 FullName 并将其返回给视图。

  2. PersonH​​tml 类。我什至不确定那会做什么。视图代表 ASP.NET MVC 应用程序中的 HTML。您在这里有两个选择:

    一个。您可以为您的模型创建一个显示模板。这是显示模板的 Stack Overflow 问题的链接,如何在 MVC 4 项目中制作显示模板

    湾。您还可以编写一个 HtmlHelper 方法来为您生成正确的 HTML。@Html.DisplayNameLink(...) 之类的东西将是您的最佳选择。这是了解 HtmlHelpers 的链接https://download.microsoft.com/download/1/1/f/11f721aa-d749-4ed7-bb89-a681b68894e6/ASPNET_MVC_Tutorial_9_CS.pdf

于 2017-01-16T15:55:29.853 回答
2

我自己也为此苦苦挣扎。当我在比 HTML 更基于逻辑的视图中编写代码时,我创建了 HtmlBuilder 的增强版本。我扩展了某些域对象以自动打印出这个助手,它的内容基于域函数,然后可以打印到视图上。然而,代码变得非常混乱和不可读(尤其是当你试图弄清楚它来自哪里时);由于这些原因,我建议从域中删除尽可能多的表示/视图逻辑。

然而,在那之后,我决定再看一下显示和编辑器模板。而且我越来越欣赏它们,尤其是与 T4MVC、FluentValidation 和自定义元数据提供程序等结合使用时。我发现使用 HtmlHelpers 并将元数据或路由表扩展到更清洁的做事方式,但您也开始使用文档较少的系统。但是,这种情况比较简单。

因此,首先,我将确保您为该实体定义了一个路由,这看起来就像您使用默认 MVC 路由一样,因此您可以在视图中简单地执行此操作:

//somewhere in the view, set the values to the desired value for the person you have
@{
    var id = 10; //random id
    var name = "random name";
}
//later:
<a href="@Url.Action("People", "Detail", new { id = id })"> @name  ( @id )</a>

或者,使用T4MVC

<a href="@Url.Action(MVC.People.Detail(id))"> @name ( @id )</a>

这意味着,关于视图/视图模型,它们唯一的依赖是idnamePerson我认为你现有的视图模型应该有(var id = x从上面删除那个丑陋的):

<a href="@Url.Action("People", "Detail", new { id = Model.PersonId } )"> 
    @Model.Name ( @Model.PersonId )
</a>

或者,使用 T4MVC:

<a href="@Url.Action( MVC.People.Detail( Model.PersonId ) )"> 
    @Model.Name ( @Model.PersonId )
</a>

现在,正如您所说,有几个视图使用此代码,因此您需要更改视图以符合上述要求。还有其他方法可以做到,但我的每一个建议都需要改变观点,我相信这是最干净的方法。这还有一个使用路由表的特性,这意味着如果路由系统更新了,那么更新后的 url 会在这里打印出来而无需担心,而不是在域对象中将其硬编码为 url(这取决于以特定方式设置路由系统以使该 url 工作)。

我的其他建议之一是构建一个 Html Helper,称为Html.LinkFor( c => model )或类似的东西,但是,除非您希望它根据类型动态确定控制器/动作,否则这是不必要的。

于 2017-01-25T03:56:33.020 回答
2

考虑到这种当前状态有多糟糕,因为除了感觉错误之外,它并没有真正导致理解代码或创建任何不良依赖关系的任何重大问题。

当前状态非常糟糕,不仅仅是因为 UI 代码包含在域代码中。那已经糟糕了,但这更糟。该NameIdHTML属性返回一个硬编码的链接到该人的 UI 页面。即使在 UI 代码中,您也不应该对此类链接进行硬编码。这就是目的LinkExtensions.ActionLinkUrlHelper.Action目的。

如果您更改控制器或路由,链接将会更改。LinkExtensionsUrlHelper意识到这一点,您不需要任何进一步的更改。当您使用硬编码链接时,您需要在代码中找到此类链接被硬编码的所有位置(并且您需要知道这些位置存在)。更糟糕的是,您需要更改的代码位于与依赖链相反方向的业务逻辑中。这是维护的噩梦,也是错误的主要来源。你需要改变这一点。

如果这里有最佳实践,因为这似乎是一个常见的问题/模式。

是的,有一个最佳实践,那就是在您需要指向控制器操作返回的页面的链接时使用上述LinkExtensions.ActionLink和方法。UrlHelper.Action坏消息是,这意味着您的解决方案中的多个位置都会发生变化。好消息是很容易找到这些位置:只需删除该NameIdHTML属性,错误就会弹出。除非您通过反射访问该属性。在这种情况下,您需要进行更仔细的代码搜索。

您将需要替换NameIdHTML为使用LinkExtensions.ActionLinkUrlHelper.Action创建链接的代码。我假设NameIdHTML返回的 HTML 代码应在此人显示在 HTML 页面上时使用。我还假设这是您代码中的常见模式。如果我的假设是正确的,您可以创建一个帮助类,将业务对象转换为它们的 HTML 表示。您可以向该类添加扩展方法,以提供对象的 HTML 表示。为了说明我的观点,我假设(假设地),你有一个Department类,它也有NameId并且有类似的 HTML 表示。然后,您可以重载转换方法:

public static class BusinessToHtmlHelper {
    public static MvcHtmlString FromBusinessObject( this HtmlHelper html, Person person) {
        string personLink = html.ActionLink(person.Name, "Detail", "People",
            new { id = person.Id }, null).ToHtmlString();
        return new MvcHtmlString(personLink + " (" + person.Id + ")");
    }

    public static MvcHtmlString FromBusinessObject( this HtmlHelper html,
        Department department) {

        string departmentLink = html.ActionLink(department.Name, "Detail", "Departments",
            new { id = department.Id }, null).ToHtmlString();
        return new MvcHtmlString(departmentLink + " (" + department.Id + ")");
    }
}

在您的视图中,您需要NameIdHTML通过调用此辅助方法来替换。例如这段代码...

@person.NameIdHTML

...需要替换为:

@Html.FromBusinessObject(person)

这也将使您的视图保持干净,如果您决定更改您的视觉表示,Person则可以轻松更改BusinessToHtmlHelper.FromBusinessObject而不更改任何视图。此外,生成的链接会自动反映对路由或控制器的更改。UI 逻辑保留在 UI 代码中,而业务代码保持干净。

如果您想让您的代码完全不受 HTML 影响,您可以为您的人员创建一个显示模板。优点是您的所有 HTML 都包含视图,缺点是您要创建的每种类型的 HTML 链接都需要一个显示模板。Person显示模板看起来像这样:

@model Person

@Html.ActionLink(Model.Name, "Detail", "People", new { id = Model.Id }, null) ( @Html.DisplayFor(p => p.Id) )

您必须person.NameIdHTML用此代码替换您的引用(假设您的模型包含Persontype 的属性Person):

@Html.DisplayFor(m => m.Person)

您也可以稍后添加显示模板。您可以创建BusinessToHtmlHelper第一个和作为将来的第二个重构步骤,在引入显示模板后更改帮助程序类(如上面的那个):

public static class BusinessToHtmlHelper {
    public static MvcHtmlString FromBusinessObject<T>( this HtmlHelper<T> html, Person person) {
        return html.DisplayFor( m => person );
    }
    //...
}

如果您只小心使用由 创建的链接BusinessToHtmlHelper,您的视图将不需要进一步更改。

于 2017-01-25T14:43:39.747 回答
1

要为这个问题提供一个完美的答案并不容易。尽管层的完全分离是可取的,但它通常会导致很多无用的工程问题。

尽管每个人都可以接受业务层对表示层/UI 层了解不多的事实,但我认为知道这些层确实存在是可以接受的,当然没有太多细节。

一旦你声明了这一点,你就可以使用一个未被充分利用的接口:IFormattable。这是string.Format使用的接口。

因此,例如,您可以首先像这样定义您的 Person 类:

public class Person : IFormattable
{
    public string Id { get; set; }
    public string Name { get; set; }

    public override string ToString()
    {
        // reroute standard method to IFormattable one
        return ToString(null, null);
    }

    public virtual string ToString(string format, IFormatProvider formatProvider)
    {
        if (format == null)
            return Name;

        if (format == "I")
            return Id;

        // note WebUtility is now defined in System.Net so you don't need a reference on "web" oriented assemblies
        if (format == "A")
            return string.Format(formatProvider, "<a href='/People/Detail/{0}'>{1}</a>", WebUtility.UrlEncode(Id), WebUtility.HtmlDecode(Name));

        // implement other smart formats

        return Name;
    }
}

这并不完美,但至少,您将能够避免定义数百个指定属性,并将演示详细信息保存在专门用于演示详细信息的 ToString 方法中。

通过调用代码,您可以像这样使用它:

string.Format("{0:A}", myPerson);

或使用 MVC 的HtmlHelper.FormatValue。.NET 中有很多类支持 IFormattable(例如 StringBuilder)。

您可以优化系统,并改为执行以下操作:

    public virtual string ToString(string format, IFormatProvider formatProvider)
    {
        ...
        if (format.StartsWith("A"))
        {
            string url = format.Substring(1);
            return string.Format(formatProvider, "<a href='{0}{1}'>{2}</a>", url, WebUtility.UrlEncode(Id), WebUtility.HtmlDecode(Name));
        }
        ...
        return Name;
    }

你会像这样使用它:

string.Format("{0:A/People/Detail/}", person)

因此,您无需在业务层中对 url 进行硬编码。将 Web 作为表示层时,您通常必须以格式传递 CSS 类名称以避免业务层中的硬编码样式。事实上,你可以想出相当复杂的格式。毕竟,如果您考虑一下,这就是对DateTime等对象所做的事情。

您甚至可以更进一步并使用一些环境/静态属性来告诉您是否在 Web 上下文中运行,以便它自动运行,如下所示:

public class Address : IFormattable
{
    public string Recipient { get; set; }
    public string Line1 { get; set; }
    public string Line2 { get; set; }
    public string ZipCode { get; set; }
    public string City { get; set; }
    public string Country { get; set; }

    ....

    public virtual string ToString(string format, IFormatProvider formatProvider)
    {
        // http://stackoverflow.com/questions/3179716/how-determine-if-application-is-web-application
        if ((format == null && InWebContext) || format == "H")
            return string.Join("<br/>", Recipient, Line1, Line2, ZipCode + " " + City, Country);

        return string.Join(Environment.NewLine, Recipient, Line1, Line2, ZipCode + " " + City, Country);
    }
}
于 2017-01-22T08:19:29.593 回答
1

理想情况下,您需要重构代码以使用视图模型。视图模型可以具有用于简单字符串格式化的实用方法,例如

public string FullName => $"{FirstName} {LastName}"

但严格来说没有 HTML!(做一个好公民:D)

然后,您可以在以下目录中创建各种编辑器/显示模板:

Views/Shared/EditorTemplates
Views/Shared/DisplayTemplates

以模型对象类型命名模板,例如

AddressViewModel.cshtml

然后,您可以使用以下内容来呈现显示/编辑器模板:

@Html.DisplayFor(m => m.Address)
@Html.EditorFor(m => m.Address)

如果属性类型是 AddressViewModel,那么将使用 EditorTemplates 或 DisplayTemplates 目录中的 AddressViewModel.cshtml。

您可以通过将选项传递给模板来进一步控制渲染,如下所示:

@Html.DisplayFor(m => m.Address, new { show_property_name = false, ... })

您可以像这样在模板 cshtml 文件中访问这些值:

@ {
    var showPropertyName = ViewData.ContainsKey("show-property-name") ? (bool)ViewData["show-property-name] : true;
    ...
}

@if(showPropertyName)
{
    @Html.TextBoxFor(m => m.PropertyName)
}

这提供了很大的灵活性,而且还可以通过将 UIHint 属性应用于属性来覆盖使用的模板,如下所示:

[UIHint("PostalAddress")]
public AddressViewModel Address { get; set; }

DisplayFor/EditorFor 方法现在将查找“PostalAddress.cshtml”模板文件,它只是另一个模板文件,如 AddressViewModel.cshtml。

对于我从事的项目,我总是将 UI 分解为这样的模板,因为您可以通过 nuget 将它们打包并在其他项目中使用它们。

此外,您还可以将它们添加到一个新的类库项目中,并将它们编译成一个 dll,您可以在 MVC 项目中引用它。我以前使用过 RazorFileGenerator 来执行此操作(http://blog.davidebbo.com/2011/06/precompile-your-mvc-views-using.html),但现在更喜欢使用 nuget 包,因为它允许对意见。

于 2017-01-25T13:21:45.627 回答
1

我想你需要在改变它之前制定一个计划。是的,你提到的项目听起来不正确,但这并不意味着新计划更好。

首先,现有项目(这将帮助您了解要避免的情况):

DomainObjects 包含数据库表?听起来像达尔。我假设这些对象实际上存储在数据库中(例如,如果它们是实体框架类)而不是从它们映射(例如,使用实体框架,然后将结果映射回这些对象),否则你有太多的映射(1从 EF 到数据对象,2 从数据对象到模型)。我已经看到这样做了,非常典型的分层错误。因此,如果您有,请不要重复。此外,不要将包含数据行对象的项目命名为 DomainObjects。领域意味着模型。

DomainORM - 好的,但我会将它与数据行对象结合起来。如果它与数据对象紧密耦合,那么将映射项目分开是没有意义的。这就像假装你可以替换一个而没有另一个。

模型 - 好名字,它也可以提到域,那样没有人会用这个非常重要的词来命名其他项目。

NameIdHTML 属性 - 业务对象的坏主意。但这是一个小的重构 - 将它移到一个方法中,该方法会留在其他地方,而不是在您的业务逻辑中。

看起来像 DTO 的业务对象 - 也是个坏主意。那么业务逻辑的意义何在?我自己的文章:如何设计业务对象

现在,您需要定位什么(如果您准备重构):

业务逻辑托管项目应该独立于平台——不提及 HTML、HTTP 或任何与具体平台相关的内容。

DAL - 应该引用业务逻辑(而不是其他方式),并且应该负责映射以及保存数据对象。

MVC - 通过将逻辑移出业务逻辑(其中逻辑实际上是业务逻辑)或所谓的服务层(又名应用程序逻辑层 - 可选并且如果需要将应用程序特定代码从控制器中取出)来保持瘦控制器。

我自己关于分层的文章:分层软件架构

这样做的真正原因:

在可能的多个平台上可重用的业务逻辑(今天你只是 web,明天你可以是 web 和服务,或者桌面)。理想情况下,所有不同的平台都应该使用相同的业务逻辑,如果它们属于相同的有界上下文。

从长远来看,可管理的复杂性是众所周知的选择 DDD(域驱动)与数据驱动设计之类的因素。它带有学习曲线,因此您最初会对其进行投资。从长远来看,您可以保持较低的可维护性,就像永久收取保费一样。当心你的对手,他们会争辩说这与他们一直在做的完全不同,而且对他们来说似乎很复杂(由于学习曲线和迭代思考以长期保持良好的设计)。

于 2017-01-25T19:08:27.397 回答
0

首先考虑您的目标和 Kent Beck 关于软件开发经济学的观点。您的软件的目标可能是提供价值,您应该将时间花在做有价值的事情上。

其次,戴上你的软件架构师的帽子,做一些计算。这就是您支持选择将资源用于此方面或用于其他方面的方式。

如果在接下来的 2 年内它会:

  • 增加不满意客户的数量
  • 减少公司的收入
  • 增加软件故障/错误/崩溃的数量
  • 增加维护或更改代码的成本
  • 让开发人员感到惊讶,导致他们在误解中浪费数小时的时间
  • 增加新开发人员的入职成本

如果这些事情不太可能由于代码而发生,那么不要将您的团队的生命浪费在拉直铅笔上。如果您无法确定代码的真正负面成本后果,那么代码可能没问题,应该改变的是您的理论。

我的猜测是“更改此代码的成本可能高于它引起的问题的成本。” 但你更适合猜测问题的实际成本。在您的示例中,更改成本可能非常低。将此添加到您的选项 2 重构列表中:

———————————————————————————————————————</p>

2c。使用 MVC 应用程序中的扩展方法以最少的代码将演示知识添加到域对象。

public static class PersonViewExtensions
{
    const string NameAndOnePhoneFormat="{0} ({1})";
    public static string NameAndOnePhone(this Person person)
    {
        var phone = person.MobilePhone ?? person.HomePhone ?? person.WorkPhone;
        return string.Format(NameAndOnePhoneFormat, person.Name, phone);
    }
}

在您嵌入 HTML 的地方,@Sefe 的答案中的代码——在类上使用扩展方法HtmlHelper——几乎就是我会做的。这样做是 Asp.NetMVC 的一大特色

——————————————————————————————————————————</p>

但是这种方法应该是整个团队的学习习惯。不要向你的老板询问重构的预算。向你的老板询问学习预算:书籍、编写代码的时间、带团队参加开发人员聚会的预算。

不管你做什么,都不要做业余软件架构的想法,“这个代码不符合 X,因此我们必须花费时间和金钱来修改它,即使我们无法证明这笔费用的具体价值。 ”</p>

最终,您的目标是增加价值。花钱学习会增加价值;花钱提供新功能或消除错误可能会增加价值;只有在真正消除缺陷的情况下,花钱重写工作代码才能增加价值。

于 2017-01-26T01:14:55.017 回答