[RESOLVED] [2005] Is there a better way of doing this If/Then loop?

[Ginolard]

I have a ListView that displays some items (it's not important what they are really). However, what IS important is that the items it displays can have a property of "IsEnabled" or not. By default I only want to show the items where IsEnabled is true. I have therefore provided a checkbox for the user to allow him/her to show all the items, enabled or not.

Now, what I want to do is to show the items that are Disabled in red and enabled items in black. The code below works but it just doesn't look right to me. Is there a better way?

The code is called everytime the CheckBox is clicked.

Private Sub GetPackages()
        Me.Cursor = Cursors.WaitCursor
        Me.lv_Packages.Items.Clear()
        Try
            objAltirisClient = CreateObject("Altiris.AeXNSClient", ComputerName)
        Catch ex As Exception
            MessageBox.Show("There was an error connecting to the Altiris Client.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
            Me.Cursor = Cursors.Default
            Return
        End Try
        objSWDAgent = objAltirisClient.ClientPolicyMgr.ClientAgent("Altiris.SWD")
        Dim ColAdvertisements As Object = objSWDAgent.Advertisements
 
        For Each objAdvert As Object In ColAdvertisements
 
            Dim LVI As New ListViewItem
            LVI.Tag = objAdvert.ID
            LVI.Text = objAdvert.Name
 
            If Me.check_ShowDisabledPackages.Checked = False Then
 
                If objAdvert.IsEnabled = 1 Then
                    Me.lv_Packages.Items.Add(LVI)
                End If
            Else
                If objAdvert.IsEnabled = 1 Then
                    Me.lv_Packages.Items.Add(LVI)
                Else
                    LVI.ForeColor = Color.Red
                    Me.lv_Packages.Items.Add(LVI)
                End If
            End If
        Next
End Sub

[Half]

So it doesn't matter if the checkbox is checked? This spares an If.

[Ginolard]

If it's unchecked it should only list Enabled Packages. If it's checked it should list Disabled and Enabled Packages.

On Form Load, it's unchecked.

[Ginolard]

bump

[Dnereb]

You could alter it into:

If Me.check_ShowDisabledPackages.Checked = False And objAdvert.IsEnabled = 1 Then
    Me.lv_Packages.Items.Add(LVI)
ElseIf objAdvert.IsEnabled = 1 Then
   Me.lv_Packages.Items.Add(LVI)
Else  ' if objAdvert.IsEnabled <> 1 I'm not sure you need this part
  LVI.ForeColor = Color.Red
  Me.lv_Packages.Items.Add(LVI)
End If

[jmcilhinney]

Why are you comparing the IsEnabled property to 1? Is it an Integer or a Boolean?

Assuming that it's a Boolean, as it should be, then this:

If Me.check_ShowDisabledPackages.Checked = False Then
 
                If objAdvert.IsEnabled = 1 Then
                    Me.lv_Packages.Items.Add(LVI)
                End If
            Else
                If objAdvert.IsEnabled = 1 Then
                    Me.lv_Packages.Items.Add(LVI)
                Else
                    LVI.ForeColor = Color.Red
                    Me.lv_Packages.Items.Add(LVI)
                End If
            End If

can be written much more efficiently like this:

If objAdvert.IsEnbaled Then
    'The object is enabled so show the item regardless of the check box.
    Me.lv_Packages.Items.Add(LVI)
Else If Me.check_ShowDisabledPackages.Checked Then
    'The item is not enabled but disabled items are to be shown.
    LVI.ForeColor = Color.Red
    Me.lv_Packages.Items.Add(LVI)
End If

[jmcilhinney]

Actually, looking more closely the whole loop can be done better. There's no point creating items that aren't going to be used:

Me.lv_Packages.BeginUpdate()
 
For Each objAdvert As Object In ColAdvertisements
    If objAdvert.IsEnabled OrElse Me.check_ShowDisabledPackages.Checked
        Dim lvi As New ListViewItem(CStr(objAdvert.Name))
 
        lvi.Tag = objAdvert.ID
 
        If Not objAdvert.IsEnabled Then
            lvi.ForeColor = Color.Red
        End If
 
        Me.lv_Packages.Items.Add(lvi)
    End If
Next objAdvert
 
Me.lv_Packages.EndUpdate()

Also, you've got some serious late-binding happening there. What's up with all the 'Object' types?

[Ginolard]

The IsEnabled property is an integer.

Yes, I know about the Late Binding issue. The problem is that I am trying to work with the Altiris API interface but the documentation is...well, not very extensive. There is a scripting API which you normally would use with VBScript/Javascript but it seems to work just as well like this.

It's not ideal admittedly and I'm sure there's a better way but I'm still trying to figure out the Altiris SDK.

Thanks for the help with the loop though. I've gone with a DataGridView now so the BeginUpdate/EndUpdate thing is no longer valid.