Control Tree – Daily WTF

User interfaces tend to map quite naturally trees as a data structure. In a modern world where we’ve forgotten how to create native interfaces and do everything as web apps pretending to be native, the “tree” model is explicit through the DOM. But even in native user interfaces, we tend to group controls into containers, panels, and tabs.

Now, there may be times when we need to traverse a tree, and trees provide us with a very natural way to traverse them – recursion. To search a tree, first search a subtree. Recursion can have its problems, but the native application’s UI tree is rarely deep enough for these problems to be a problem.

Nick inherited some VB .Net code that iterates over all the controls on the screen. It does not use recursion, which in itself is not a problem. Any other choice in the code is a problem.

For Each x In Me.Controls
    Select Case (x.GetType.ToString)
        Case "System.Windows.Forms.Panel"
            For Each y In x.Controls
                Select Case (y.GetType.ToString)
                    Case "System.Windows.Forms.TabControl"
                        For Each z In y.Controls
                            Select Case (z.GetType.ToString)
                                Case "System.Windows.Forms.TabPage"
                                    For Each w In z.Controls
                                        If w.Tag = "Store Info" Then
                                            strName = CheckForExceptions(w.Name)
                                            iIndexStoreInfoValues = (arrStoreInfoValues.Length - 1)
                                            If w.GetType().ToString() = "System.Windows.Forms.CheckBox" Then
                                                arrStoreInfoValues(iIndexStoreInfoValues) = CType(w, CheckBox).CheckState
                                            Else
                                                arrStoreInfoValues(iIndexStoreInfoValues) = w.Text
                                            End If
                                            ReDim Preserve arrStoreInfoValues(iIndexStoreInfoValues + 1)
                                        End If
                                    Next
                            End Select
                        Next
                    Case "System.Windows.Forms.GroupBox"
                        For Each z In y.Controls
                            If z.Tag = "Store Info" Then
                                iIndexStoreInfoValues = (arrStoreInfoValues.Length - 1)
                                arrStoreInfoValues(iIndexStoreInfoValues) = z.Text
                                ReDim Preserve arrStoreInfoValues(iIndexStoreInfoValues + 1)
                            End If
                        Next
                    Case Else
                        If y.Tag = "Store Info" Then
                            iIndexStoreInfoValues = (arrStoreInfoValues.Length - 1)
                            strName = CheckForExceptions(y.Name)
                            arrStoreInfoValues(iIndexStoreInfoValues) = y.Text
                            ReDim Preserve arrStoreInfoValues(iIndexStoreInfoValues + 1)
                        End If
                End Select
            Next
        Case Else 
            For Each y In x.Controls
                If y.Tag = "Store Info" Then
                    iIndexStoreInfoValues = (arrStoreInfoValues.Length - 1)
                    arrStoreInfoValues(iIndexStoreInfoValues) = y.Text
                    ReDim Preserve arrStoreInfoValues(iIndexStoreInfoValues + 1)
                End If
            Next
     End Select
Next

At its deepest, this stack of code has 12 levels of indentation. Which I cleaned up a bit here – as submitted, the code used 8 spaces per tab, which made it completely unreadable in a window of reasonable width. Not that it would be readable at any width, mind you.

I won’t go through this code line by line. It’s not worth the psychological damage I’d cause you. But I want to make a few points.

Note the repeated uses ReDim Preserve arrStoreInfoValues.... This is an ugly VB hack to pretend you have arrays that can grow- ReDim creates a completely new array, new sizes and Preserve ensures that all data in the original array is copied to the new array. It’s expensive and time-consuming, and .NET has many built-in data structures that make this unnecessary. It’s probably inherited from some VB6 code that was ported to .NET. It was a strangely common convention in VB6, instead of using it any type of data structure that can grow.

In a few cases we use a Select with only one branch to it – which makes it fancy If statement but less readable. I suppose there is good logic in that you may want more branches at some point, but I still hate reading it. Even worse, those switch statements are included string representation of the control type. That particular code stink is always “fun” to see and hints at an easy way to clean up this code: refactor those branches into methods and use function overloading and the type system to replace switches – if that was even necessary, since many branches do basically the same thing anyway.

But there’s no point in cleaning up this code, because as Nick discovered: it didn’t actually work. The assumptions that code makes about how controls are organized are wrong assumptions. It misses a lot of controls in its tour.

Nick replaced it with a 10-line recursive function that never needs to be checked GetType any of the controls.