Another common Anti-pattern is the Arrowhead Antipattern. Refactoring away from the arrowhead antipattern is as simple as swapping the conditionals, i.e. flatten conditionals statements, to leave the method as soon as possible. It is so common and so much have been written about it, I thought to just copy the links:
- http://www.codinghorror.com/blog/archives/000486.html
- http://www.lostechies.com/blogs/sean_chambers/archive/2009/08/24/refactoring-day-24-remove-arrowhead-antipattern.aspx
Sometimes, the Arrowhead Antipattern is not clearly visible, or the automated refactoring tools fails to do the refactoring, due to the else condition, etc.
if (listView.SelectedItems.Count > 0)
{
ListViewItem item = listView.SelectedItems[0];
if (item != null)
{
listView.Items.Remove(item);
}
}
else
MessageBox.Show(this, "Please select an item from the list to remove.");
{
ListViewItem item = listView.SelectedItems[0];
if (item != null)
{
listView.Items.Remove(item);
}
}
else
MessageBox.Show(this, "Please select an item from the list to remove.");
But whenever you see some validation code and the Arrowhead Antipattern, it means, time to refactoring. Most the time, reversing the if condition will do the job, and the automated tools will now identify that piece of code as a candidate for refactoring Arrowhead Antipattern.
if (listView.SelectedItems.Count <= 0)
{
MessageBox.Show(this, "Please select an item from the list to remove.");
return;
}
ListViewItem item = listView.SelectedItems[0];
if (item == null)
return;
listView.Items.Remove(item);
{
MessageBox.Show(this, "Please select an item from the list to remove.");
return;
}
ListViewItem item = listView.SelectedItems[0];
if (item == null)
return;
listView.Items.Remove(item);
