CodeSOD: When All You Have Is .Sort, Every Problem Looks Like a List(of String)
When it comes to backwards compatibility, Microsoft is one of those vendors that really commits to it. It's not that they won't make breaking changes once in awhile, but they recognize that they need to be cautious about it, and give customers a long window to transition.
This was true back when Microsoft made it clear that .NET was the future, and that COM was going away. To make the transition easier, they created a COM Interop system which let COM code call .NET code, and vice versa. The idea was that you would never need to rewrite everything from scratch, you could just transition module by module until all the COM code was gone and just .NET remained. This also meant you could freely mix Visual Basic and Visual Basic.Net, which never caused any problems.
Well Moritz sends us some .NET code that gets called by COM code, and presents us with the rare case where we probably should just rewrite everything from scratch.
''' <summary> ''' Order the customer list alphabetically ''' </summary> ''' <returns></returns> ''' <remarks></remarks> Public Function orderCustomerAZ() As Boolean Try Dim tmpStrList As New List(Of String) Dim tmpCustomerList As New List(Of Customer) ' We create a list of ID strings and order it For i = 0 To CustomerList.Count - 1 tmpStrList.Add(CustomerList(i).ID) Next i tmpStrList.Sort() ' We create the new tmp list of customers For i = 0 To tmpStrList.Count - 1 For j = 0 To CustomerList.Count - 1 If CustomerList(j).ID = tmpStrList(i) Then tmpCustomerList.Add(CustomerList(j).Clone) Exit For End If Next j Next i ' We update the list of customers CustomerList.Clear() CustomerList = tmpCustomerList Return True Catch ex As Exception CompanyName.Logging.ErrorLog.LogException(ex) Return False End Try End Function
As the name implies, our goal is to sort a list of customers... by ID. That's not really implied by the name. The developer responsible knew how to sort a list of strings, and didn't feel any need to learn what the correct way to sort a list of objects were.
So first, they build a tmpStrList which holds all their IDs. Then they Sort() that.
Now that the IDs are sorted, they need to organize the original data in that order. So they compare each element of the sorted list to each element of the unsorted list, and if there's a match, copy the element into tmpCustomerList, ensuring that list holds the elements in the sorted order.
Finally, we clear out the original list and replace it with the sorted version. Return True on success, return False on failure. This last bit makes the most sense: chucking exceptions across COM Interop is fraught, so it's easier to just return status codes.
Everything else though is a clear case of someone who didn't want to read the documentation. They knew that a list had a Sort method which would sort things like numbers or strings, so boom. Why look at all the other ways you can sort lists? What's a comparator" or a lambda? Seems like useless extra classes.
[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!