ASP.NET somewhat emulates the MVC architecture with its code behind web forms. For those who are not familiar, each web form consists of two files; The ASPX file used for defining the appearance of the page (View), and a code behind file is used to implement the page logic (Controller).
For the most part, it is easy to keep a clean separation of concerns between the two files. However, I repeatedly see inline code mixed in with presentation when people are using data bound controls.
An example would be the ASP:Repeater control. In a nutshell, the repeater control allows you to create a template and populate it with data from any databindable source (Be it a DataSet, a DataReader, or even a custom collection).
A common implementation of the ASP:Repeater would be similar to this code from an early version of CodeBlog:
1 <asp:Repeater ID="rptrComment" runat="server">
2 <ItemTemplate>
3 <div class="comment-body">
4 <%# DataBinder.Eval(Container.DataItem, "CommentText") %>
5 <div class="comment-name">
6 <a href="<%# DataBinder.Eval(Container.DataItem,"AuthorWeb") %>">
7 <%# DataBinder.Eval(Container.DataItem, "AuthorName") %>
8 </a>
9 - <%# DataBinder.Eval(Container.DataItem, "ShortDateTime") %>
10 </div>
11 </div>
12 </ItemTemplate>
13 </asp:Repeater>
This repeater was bound to the Comment custom collection I wrote from CodeBlog's business objects. While it works flawlessly, we are embedding inline code to loosely bind our collection to the fields we want outputted.
The ItemDataBound event is fired every time the repeater binds an item. By catching this event, we can move all of our code to the code behind file where it belongs. We can also add some logic (such as suppressing the hyper link if we don't have a website in the comment object).
1 <asp:Repeater ID="rptrComment" runat="server">
2 <ItemTemplate>
3 <div class="comment-body">
4 <asp:Literal ID="litCommentText" runat="server" />
5 <div class="comment-name">
6 <asp:HyperLink id="hplCommentAuthor" runat="server">
7 <asp:Label ID="lblCommentAuthorName" runat="server" />
8 </asp:HyperLink>
9 - <asp:Label ID="lblCommentDateTime" runat="server" />
10 </div>
11 </div>
12 </ItemTemplate>
13 </asp:Repeater>
14
The main difference between the two code snippets is that the inline code has been replaced with ASP.NET Server controls. We will populate these controls on each ItemDataBound event. With the inline code gone, the Repeater is much more concise and this page will be easier to maintain in the future.
In the code behind file, the first task is to capture the ItemDataBound event for the repeater. This is done by overriding OnInit (if you haven't already for some other reason) and adding the event handler callback:
1 protected override void OnInit(EventArgs e)
2 {
3 base.OnInit(e);
4 rptrComment.ItemDataBound += new RepeaterItemEventHandler(rptrCommentsItemDataBound);
5 }
With this done, we can implement a method to handle the event:
1 private void rptrCommentsItemDataBound(Object Sender, RepeaterItemEventArgs e)
2 {
3 // This event will be fired even for Header, Seperator, and Footer rows, so we ignore those
4 if (e.Item.ItemType == ListItemType.Item || e.Item.ItemType == ListItemType.AlternatingItem)
5 {
6
7 // Item.DataItem contains the Item we are binding, in this case, the custom Business Object Comment
8 // A simple cast is all that it takes to convert Item.DataItem into a Comment.
9 Comment currentComment = (Comment) e.Item.DataItem;
10
11 // We have to create references to the controls in the Repeater using FindControl
12 Literal CommentText = (Literal)e.Item.FindControl("litCommentText");
13 Label CommentAuthorName = (Label)e.Item.FindControl("lblCommentAuthorName");
14 Label CommentDateTime = (Label)e.Item.FindControl("lblCommentDateTime");
15 HyperLink CommentAuthor = (HyperLink)e.Item.FindControl("hplCommentAuthor");
16
17 // Populate the Controls with Data from the Comment Object
18 CommentText.Text = currentComment.Text;
19 CommentAuthorName.Text = currentComment.AuthorName;
20 CommentDateTime.Text = currentComment.ShortDateTime;
21
22 // A little logic allows us to only populate the Hyperlink of we have a website stored.
23 if (currentComment.AuthorWeb != string.Empty)
24 {
25 CommentAuthor.NavigateUrl = currentComment.AuthorWeb;
26 }
27 }
28 }
The code comments explain the technique fairly well. To summorize, each time the event is fired, Item.DataItem returns a unique Comment object. At that point, it is a trivial matter of setting the Server controls properties with information obtained by the object.
By making these small changes, I have moved all of the logic into the code behind file where it belongs making ASPX file easier to follow. I was also able to add some conditional logic on the databinding, and the result was a very easy to understand event handler. Overall, this creates clean code that is easy to understand and even easier to maintain.
There is never a reason to use inline code when you are databinding. Keep a clean separation of concerns and keep your presentation layer and logic layer where they belong.
7 comments:
The aspx is cleaner and you do have compile time checking on the dataitems fields now I guess. Just a pity about the magic strings in the FindControl method.
You do realize you didn't actually do anything at all with the "refactoring", right?
You actually made the problem worse. The inline code will databind to anything with matching properties. Your code-behind has an explicit dependency on a concrete object (and worse, you add some more objects in the form of controls that you also added dependencies to). How's that better?
Anonymous-
This repeater is to bind to one collection type, and one only.
As Betty said, I have added compile time constraints with this method so that it can only bind to the Comment object, which is what I desire.
Using Databinder.Eval uses late binding based upon Reflection calls, which is not only slow, but prone to runtime errors that are hard to detect.
Instead of binding to some magic fields we now have strong a strong typed collection. It is a shame about using FindControl, but I'm not sure if any other way around that.
I posted my comments here.
But why use DataBinder.Eval at all? You mention the strongly typed collection binding as an advantage (which it surely is), but why not just use that in the ASPX? Much cleaner, much less error prone as you get compile time checking.
DataBinder.Eval(Container.DataItem, "ShortDateTime")
=>
((Comment)Container.DataItem).ShortDateTime
Now the only thing missing is the possibility of a null website address, but it's a lot neater to just make a function for this, than to create your example function.
Helpfull, just what i was looking for! About((Comment)Container.DataItem).ShortDateTime
The problem is that it will not complain compiletime when you make changes to the comment class or mistakes like:((Comment)Container.DataItem).bogus
This is misleading because intellisense knows enough to give you the impression that the compiler would give an error when using a wrong name. The compiler however ignores all inline code.
I'll go for a method that gives me the text like <%#GetText(Container)%>.
The findcontrol() method seems expensive.
I generally prefer keeping everything in the markup as it makes things much easier to change, especially when you work with a front end developer who does not know C# but can write HTML.
Also you don't need to use DataBinder.Eval(Container.DataItem, "ShortDateTime" - you can simply use Eval("ShortDateTime") (in asp.net 2.0).
Post a Comment