Nora found a curious little C# helper method. It was called in exactly one place and it is… not exactly necessary in any case.
/// <summary>
/// Return the Filter Index
/// </summary>
/// <param name="item"></param>
/// <returns></returns>
private int GetFilterIndex(bool item) {
int index = 0;
switch (item) {
case true:
index = 1;
break;
default:
index = 0;
break;
}
return index;
}
This method wants to take a boolean and convert it into an integer- 1
if true, 0
if false. Right off the bat, this method isn't needed- Convert.ToInt32
would do the job. Well, maybe our developer didn't know about that method, which isn't really an excuse, but then they write this logic in the most verbose way possible, favoring a switch over a simpler conditional, doing the dance of "only one return from a method" (which I'm not a fan of as a rule).
Even so, you could compact this into an actually readable ternary: return item ? 1 : 0;
So let's just see how this unnecessary method gets invoked.
FilterIndex = GetFilterIndex(FilterCheckBox.Checked);
FilteredSortedItems = FilterIndex > 0 ? CurrentItemList.Where(info => Convert.ToInt32(info.IsFlagged) == FilterIndex).ToList() : CurrentItemList.ToList();
Okay, so they do know about the Convert.ToInt32
method. Good to know. But then their use of GetFilterIndex
is to do a comparison to see if it`s greater than 0, which only happens if it's true, so there's no need for this filter index method at all.
FilterdSortedItems = FilterCheckBox.Checked ? …
Oh, and the Convert.ToInt32
used here is also unnecessary, since info.IsFlagged
is also a boolean, so once again, we could just compare info.IsFlagged == FilterCheckBox.Checked
.
It's a method written the long way around to solve a problem in one place that doesn't even need to be solved, and as a bonus, the use of the method, its name, and its role, just makes the code less clear and harder to understand.
![](https://thedailywtf.com/images/inedo/buildmaster-icon.png)