1

(I hope this isn't regarded as "too much like a discussion", but we'll see.)

I have a few hours to refactor long-existing code. I notice I have quite a number of static methods on ListItemCollection:

public static ListItem ListItem_InsertIfNotPresent(ListItemCollection lic, ...)
public static ListItemCollection ListItems_Selected(ListItemCollection lic)
public static string[] ListItemValues(ListItemCollection lic)
public static void ListItems_SortByText(ListItemCollection lic)
....

So they look suitable to refactor. ListItemCollections are returned by many CLR functions, so I cannot derive/sub-class. I am toying with:

  1. Implement as extension methods. Code currently does not have any extension methods. Not mad about having to add using ListItemCollectionExtensions to all my source files, nor risking future .NET CLRs possibly adding clashes. But the calling syntax is nice.
  2. Add a ListItemCollectionHelper class to put these in. Code will have to go var licx = new ListItemCollectionHelper(dropDownList.Items); before any licx.SortByText();-type calls, which is a bit messy when I only need to call one method.
  3. Leave as-is, with named statics as now!

I have looked at articles like What are the best practices for using Extension Methods in .Net? and others, but they don't really advise this case. Of particular interest is your comments on #1 versus #2.

I don't actually have to worry about "other users" of my code, but good practice never hurts. For right or for wrong, I do not want to use LINQ or lambda solutions.

Community
  • 1
  • 1
JonBrave
  • 4,045
  • 3
  • 38
  • 115

2 Answers2

1

In your example I would leave it as is unless you need the code somewhere else because it is working and refactoring just to refactor might lead to bugs. However, if I was starting from scratch I would make these extension methods. It makes the code easier to re-use and hopefully prevents someone from copying this code the next time this same pattern is needed.

I prefer Extension methods over the Helper class because it is generally easier to find the methods through Intellisense. In my experience any class that ends with the word Helper turns into a mess at some point. That becomes the place where code that does not fit anywhere goes. The Helper classes never start out bad but in a larger team you will soon find methods in there that have no business being there.

Shaun Bowe
  • 9,840
  • 11
  • 50
  • 71
  • Thank you (both) for your suggestions. At least I know that Helper classes for this are frowned upon. On this occasion I think I won't bother to refactor for the sake of it after all, as it would be the only use of Extension Methods in the code at present, but I will consider these for the future. – JonBrave Jul 15 '14 at 08:36
0

I would go ahead and make them extension methods.

  1. It doesn't break anything (you can still use like regular methods in the Helper pattern, so your existing code will still function)
  2. When you do refactor existing code or write new code, you can use the cleaner extension method syntax.
mason
  • 31,774
  • 10
  • 77
  • 121