Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have the following code and I think it's almost impossible to read and to maintain. I prefer self-explaining and modular code where variable and function names tell what I'm trying to do, but I don't know how to change the part using Eval and inside an ascx.

I'm doing much research but I still cannot understand it completely well. The performance of this part is not critical, the readability is!

How can I refactor it and split into many variable assignments?

<tr class="GridRowE" visible='<%#((String.IsNullOrEmpty(Eval("IDLanguage").ToString()) OrElse Eval("IDLanguage").ToString().Replace(" ", "").Split(","c).Contains(WebProfile.Current.IDLanguage.ToString())) And (String.IsNullOrEmpty(Eval("Roles").ToString()) OrElse Roles.IsUserInRole(WebProfile.Current.UserName, Eval("Roles").ToString())))%>' runat="server">
share|improve this question
up vote 2 down vote accepted

ASP.NET and code-behind

One of the main advantages of ASP.NET is the code-behind model. This ensures that the logic and the markup are separated in different files. The markup's goal is to display data, not to determine whether controls should be visible or not. So, your code is in violation with that principle.


The Evil of Eval

I suggest your read following article: The Evil of Eval() in ASP.NET. Here's one of the many reasons:

Eval takes about 1/20th of a second per call, and the other way takes about 1/1000th of a second per call. On a grid with 5 columns and 8 rows, Eval would take 2 seconds and the other would take under 1 second. Nobody would notice the difference. However, change that grid to 10 columns and 50 rows. You now have a difference of 25 seconds. It is the kind of thing that might not matter very much, most of the time but when it does, it makes a big difference.

The article contains more information and links to other resources.


Solution

Make the tr server-side by adding runat="server" to to the tag and give it an ID. Now you can access the tr-tag from the code-behind and set it's visibility property. Example:

Markup

<tr id="myRow" runat="server">
    <td>This is not visible</td>
</tr>

Code-behind

Dim isVisible As Boolean = False
'apply logic to determine whether "isVisible" should be true or false
myRow.Visible = isVisible

Solution for the Repeater - ItemTemplate

Here's a solution that works when your data resides in an ASP.NET Repeater control. Following is a sample markup:

<asp:Repeater ID="PeopleRepeater" runat="server">
    <HeaderTemplate>
        <!-- Your headertemplate code -->
    </HeaderTemplate>
    <ItemTemplate>
        <tr id="UserRow" runat="server">
            <!-- Your data here -->
        </tr>
    </ItemTemplate>
    <FooterTemplate>
        <!-- Your footertemplate code -->
    </FooterTemplate>
</asp:Repeater>

And here's the code that does the magic:

Sub PeopleRepeater_Bind(ByVal sender As Object, ByVal e As RepeaterItemEventArgs)
    Handles PeopleRepeater.ItemDataBound

    If e.Item.ItemType = ListItemType.Item Or
       e.Item.ItemType = ListItemType.AlternatingItem Then

        Dim isRowVisible As Boolean = False 'Your condition here
        CType(e.Item.FindControl("UserRow"), HtmlTableRow).Visible = isRowVisible

    End If

End Sub

The code checks if the current bound item is an item or an alternating item. This is necessary because the binding of the header and footer will also trigger this event. If this is the case, prepare your boolean variable to set the row visible or not. Lastly, get the table row tag using the FindControl method and set its visibility to the variable.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.