How bad is this current state considered because besides feeling wrong, it not really causing any major problems understanding the code or creating any bad dependencies.
The current state is very bad, not only because UI code is included in domain code. That would be already pretty bad, but this is worse. The NameIdHTML property returns a hardcoded link to the person's UI page. Even in UI code you should not hardcode these kind of links. That is what LinkExtensions.ActionLink and UrlHelper.Action are for.
If you change your controller or your route, the link will change. LinkExtensions and UrlHelper are aware of this and you don't need any further changes. When you use a hardcoded link, you need to find all places in your code where such a link is hardcoded (and you need to be aware that those places exist). To make matters even worse, the code you need to change is in the business logic which is in the opposite direction of the chain of dependencies. This is a maintenance nightmare and a major source of errors. You need to change this.
If there is a best practice here as it seems like this is a common problem / pattern that comes up.
Yes, there is a best practice and that is using the mentioned LinkExtensions.ActionLink and UrlHelper.Action methods whenever you need a link to a page returned by a controller action. The bad news is that this means changes at multiple spots in your solution. The good news is that it's easy to find those spots: just remove the NameIdHTML property and the errors will pop up. Unless you are accessing the property by reflection. You will need to do a more careful code search in this case.
You will need to replace NameIdHTML by code that uses LinkExtensions.ActionLink or UrlHelper.Action to create the link. I assume that NameIdHTML returns HTML code that should be used whenever this person is to be shown on an HTML page. I also assume that this is a common pattern in your code. If my assumption is true, you can create a helper class that converts business objects to their HTML representations. You can add extension methods to that class that will provide the HTML representation of your objects. To make my point clear I assume (hypothetically), that you have a Department class that also has Name and Id and that has a similar HTML representation. You can then overload your conversion method:
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 + ")");
}
}
In your views you need to replace NameIdHTML by a call to this helper method. For example this code...
@person.NameIdHTML
...would need to be replaced by this:
@Html.FromBusinessObject(person)
That would also keep your views clean and if you decide to change the visual representation of Person you can easily change BusinessToHtmlHelper.FromBusinessObject without changing any views. Also, changes to your route or controllers will be automatically reflected by the generated links. And the UI logic remains with the UI code, while business code stays clean.
If you want to keep your code completely free from HTML, you can create a display template for your person. The advantage is that all your HTML is with the views, with the disadvantage of needing a display template for each type of HTML link you want to create. For Person the display template would look something like this:
@model Person
@Html.ActionLink(Model.Name, "Detail", "People", new { id = Model.Id }, null) ( @Html.DisplayFor(p => p.Id) )
You would have to replace your references to person.NameIdHTML by this code (assuming your model contains a Person property of type Person):
@Html.DisplayFor(m => m.Person)
You can also add display templates later. You can create BusinessToHtmlHelper first and as a second refactoring step in the future, you change the helper class after introducing display templates (like the one above):
public static class BusinessToHtmlHelper {
public static MvcHtmlString FromBusinessObject<T>( this HtmlHelper<T> html, Person person) {
return html.DisplayFor( m => person );
}
//...
}
If you were careful only to use links created by BusinessToHtmlHelper, there will be no further changes required to your views.
