Matt is supporting an old VB.Net application. This representative line highlights the overall code quality:

If Me.lstRequests.GetSelected(Me.lstRequests.SelectedIndex) = False Then

lstRequests is a list box. SelectedIndex returns the currently selected index. GetSelected returns a boolean based on whether a given index is selected.

So, if there is a selected index, then GetSelected would return true, because of course the SelectedIndex is selected, because that's what SelectedIndex means.

At first, I was wondering to myself if maybe this was just an awkward way to check if something was selected. If nothing is selected, then SelectedIndex returns -1, which couldn't be selected. And maybe this is an awkward way to perform that check, but it's more awkward than it looks, as GetSelected throws an exception if "The index parameter is less than zero or greater than or equal to the value of the Count property of the ListBox.ObjectCollection class."

This code would let you know if no items were selected by throwing an exception. Maybe that's intentional, but what a terrible intent that is. And I don't think that's the case, as Matt describes the code this way: "I found some completely redundant code in an application I'm supporting"

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!