I’m starting a new line of blog posts in which I’m going to give out a few tips and tricks I’ve picked up during the past years. I’m going to start with one of the most common mistakes I often face when reading code.
To demonstrate that, I’m going to use LinqToSQL as my data access method (although the problem can be found in any kind of Data access technology) and use the the same model I used in my earlier Caching series.
As you can see the model is very simple and contains just three entities, a Peson, its Phones and its Email addresses.
Next I’m going to create a web form in which I want to display a list of Persons along with their email address. To do that I’m going to add a Repeater control which I’m going to Bind to the Person retrieved from the store.
<asp:Repeater ID="personGrid" runat="server" OnItemDataBound="personGrid_ItemDataBound"> <ItemTemplate> <asp:Label ID="lblName" runat="server" Text='<%# string.Format("{0} {1}", Eval("FirstName"), Eval("LastName")) %>'></asp:Label> | <asp:Label ID="lblEmail" runat="server" Text='Email' Font-Bold="True" Font-Italic="True"></asp:Label> </ItemTemplate> <SeparatorTemplate><br /></SeparatorTemplate> </asp:Repeater>
Since the Email Address is not in the same entity as the Person I’m going to take advantage of the repeater’s OnItemDataBound event to fetch the Email address of these persons. So typically would find something like that in the code behind.
protected void Page_Load(object sender, EventArgs e) { if (!Page.IsPostBack) { DataBind(); } } public override void DataBind() { base.DataBind(); using (AdventureworksDataContext context = new AdventureworksDataContext()) { var query = from p in context.Persons select p; personGrid.DataSource = query.Take(10); personGrid.DataBind(); } } protected void personGrid_ItemDataBound(object sender, RepeaterItemEventArgs e) { Label lblEmail = e.Item.FindControl("lblEmail") as Label; Person currentPerson = e.Item.DataItem as Person; if (lblEmail != null && currentPerson != null) { using (AdventureworksDataContext context = new AdventureworksDataContext()) { var personEmail = context.EmailAddresses.Where(ea => ea.BusinessEntityID == currentPerson.BusinessEntityID).FirstOrDefault(); lblEmail.Text = (personEmail != null) ? personEmail.EmailAddress1 : ""; } } }
This will work just fine but what some developers don’t realize is that that this will make as many queries to the database as the records on the Persons Table, cause it’s time a Person record is Data bound I’m querying for its Email Address on the ItemDataBound Event Handler.
What we could do instead, is take advantage of all the cool new features in .Net 3.5 and Linq, like data projection, data shaping and anonymous types to prepare a read-only view to bind to the repeater. So a more optimized and much cleaner version of this code would look something like that:
protected void Page_Load(object sender, EventArgs e) { if (!Page.IsPostBack) { DataBind(); } } public override void DataBind() { base.DataBind(); using (AdventureworksDataContext context = new AdventureworksDataContext()) { var query = from p in context.Persons select new { FirstName = p.FirstName, LastName = p.LastName, Email = p.EmailAddresses.Select(e => e.EmailAddress1).FirstOrDefault() }; personGrid.DataSource = query.Take(10); personGrid.DataBind(); } } //protected void personGrid_ItemDataBound(object sender, RepeaterItemEventArgs e) //{ // Label lblEmail = e.Item.FindControl("lblEmail") as Label; // Person currentPerson = e.Item.DataItem as Person; // if (lblEmail != null && currentPerson != null) // { // using (AdventureworksDataContext context = new AdventureworksDataContext()) // { // var personEmail = context.EmailAddresses.Where(ea => ea.BusinessEntityID == currentPerson.BusinessEntityID).FirstOrDefault(); // lblEmail.Text = (personEmail != null) ? personEmail.EmailAddress1 : ""; // } // } //}
This way I can also remove the OnItemDataBound event handler completely (or use it just for visual stuff that’s my preference) from the repeater and bind the Email Label to the new Field of the anonymous type that I’ve just created. So the page code is going to look something like that:
<asp:Repeater ID="personGrid" runat="server" OnItemDataBound="personGrid_ItemDataBound"> <ItemTemplate> <asp:Label ID="lblName" runat="server" Text='<%# string.Format("{0} {1}", Eval("FirstName"), Eval("LastName")) %>'></asp:Label> | <asp:Label ID="lblEmail" runat="server" Text='<%# Eval("Email") %>' Font-Bold="True" Font-Italic="True"></asp:Label> </ItemTemplate> <SeparatorTemplate><br /></SeparatorTemplate> </asp:Repeater>
Although this kind of behavior is pretty obvious and I’m pretty sure that most of you are already aware of it, there are times when this behavior is disguised and rather difficult to spot. For example the following code has exactly the same problem since the GetEmailAddress method is going to be called as many times as the person records (although there is no ItemDataBOund handler).
<asp:Repeater ID="personGrid" runat="server" OnItemDataBound="personGrid_ItemDataBound"> <ItemTemplate> <asp:Label ID="lblName" runat="server" Text='<%# string.Format("{0} {1}", Eval("FirstName"), Eval("LastName")) %>'></asp:Label> | <asp:Label ID="lblEmail" runat="server" Text='<%# GetEmailAddress((int)Eval("ID")) %>' Font-Bold="True" Font-Italic="True"></asp:Label> </ItemTemplate> <SeparatorTemplate><br /></SeparatorTemplate> </asp:Repeater>
And of course this behavior doesn’t only apply to repeaters but in every list control (drop down list, GridView, Checkbox list etc) that is going to be bound to fields belonging to more than a single entity.
thanks for sharing
This article gives the light in which we can observe the reality. this is very nice one and gives in
depth information. thanks for this nice article Good post…..Valuable information for all.I will
recommend my friends to read this for sure…
Konstantinos, did you try to count performance for this approach? I heard that linq is not that good