[RESOLVED] [2008] Better way to write this code?

[MrNorth]

Hi!

This piece of code looks really bad. Basically I want to remove the option to press a button and instead show a message, if the value in the datasource is <> -1

This code looks bad, can someone find a better and nicer way to write it?

 Dim book As HiLibrary.BooksRow

        If (e.Row.DataItem IsNot Nothing) Then
            book = CType(CType(e.Row.DataItem, System.Data.DataRowView).Row, HiLibrary.BooksRow)

            ' Om boken är utlånad till denna användaren, då kan han inte låna den igen
            If (book.IS_LOANED <> -1) Then
                ' Dölj knappen
                CType(e.Row.FindControl("Button1"), Button).Visible = False
                Dim alert As New Literal
                alert.ID = "litAlert"
                alert.Text = "Book is already loaned"
                e.Row.Cells(5).Controls.Add(alert)
            End If
        End If

kind regards
Henrik

[mendhak]

Dim book As HiLibrary.BooksRow
   If e.Row.RowType = DataControlRowType.DataRow Then
      book = CType(e.Row.DataItem, HiLibrary.BooksRow)
            If (book.IS_LOANED > -1) Then
                ' Dölj knappen
                CType(e.Row.FindControl("Button1"), Button).Visible = False
                Dim alert As New Label
                alert.ID = "AlertMessage"
                alert.Text = "Book has already been loaned"
                e.Row.Cells(5).Controls.Add(alert)
            End If
'.............

[MrNorth]

Thanks.

So it is ok to just hide the button, I was thinking about removing it completly from the tree?

Also, am I using the correct pattern for adding a new control dynamically. I just set the ID and the text. Is that enough?

Also, should I use the indexers when referring to a cell? It looks very "ASP.NET 1.1" if you know what I mean.

kind regards
Henrik

[mendhak]

There isn't a point to removing it because it's "there" already. By hiding it, you're ensuring that it's not being sent to the browser.

Also, am I using the correct pattern for adding a new control dynamically. I just set the ID and the text. Is that enough?

Yes it's enough. As an alternative, it seems like there are only a few options for each cell. You could actually have the 'not available' message present in the cell but make it invisible in the databound event. This means that you perform your check and then based upon whether or not it's available you make either the button visible or make the message visible. This way you can avoid the "very asp.net 1.1" feel, but to be honest this is how it'd be done in ASP.NET 1.1 as well.

[MrNorth]

This way you can avoid the "very asp.net 1.1" feel, but to be honest this is how it'd be done in ASP.NET 1.1 as well.

You mean ASP.NET 2.0?

Kind regards
Henrik

[mendhak]

I mean ASP.NET 2.0, yes. Across all ASP.NET versions, that's how you'd do it.