views:

295

answers:

6

We seems to be abstracting a lot of logic way from web pages and creating "helper" classes. Sadly, these classes are all sounding the same, e.g

ADHelper, (Active Directory) AuthenicationHelper, SharePointHelper

Do other people have a large number of classes with this naming convention?

A: 

I wouldn't say that it is a code smell. In ASP.NET MVC it is quite common.

Darin Dimitrov
+3  A: 

Depends on the actual content of the classes.

If a huge amount of actual business logic/business rules are in the helper classes, then I would say yes.

If the classes are really just helpers that can be used in other enterprise applications (re-use in the absolute sense of the word -- not copy then customize), then I would say the helpers aren't a code smell.

Jon Limjap
Taking the amount of business logic into account for this is a good idea. +1 for that.
OregonGhost
+1  A: 

As always, it depends on the context.

When you work with your own API I would definitely consider it a code smell, because FooHelper indicates that it operates on Foo, but the behavior would most likely belong directly on the Foo class.

However, when you work with existing APIs (such as types in the BCL), you can't change the implementation, so extension methods become one of the ways to address shortcomings in the original API. You could choose to names such classes FooHelper just as well as FooExtension. It's equally smelly (or not).

Mark Seemann
+1  A: 

It is an interesting point, if a word becomes 'boilerplate' in names then its probably a bit whiffy - if not quite a real smell. Perhaps using a 'Helper' folder and then allowing it to appear in the namespace keeps its use without overusing the word?

Application.Helper.SharePoint
Application.Helper.Authentication

and so on

amelvin
+1  A: 

In many cases, I use classes ending with Helper for static classes containing extension methods. Doesn't seem smelly to me. You can't put them into a non-static class, and the class itself does not matter, so Helper is fine, I think. Users of such a class won't see the class name anyway.

The .NET Framework does this as well (for example in the LogicalTreeHelper class from WPF, which just has a few static (non-extension) methods).

Ask yourself if the code would be better if the code in your helper class would be refactored to "real" classes, i.e. objects that fit into your class hierarchy. Code has to be somewhere, and if you can't make out a class/object where it really belongs to, like simple helper functions (hence "Helper"), you should be fine.

OregonGhost
+7  A: 

I would say that it qualifies as a code smell, but remember that a code smell doesn't necessarily spell trouble. It is something you should look into and then decide if it is okay.

Having said that I personally find that a name like that adds very little value and because it is so generic the type may easily become a bucket of non-related utility methods. I.e. a helper class may turn into a Large Class, which is one of the common code smells.

If possible I suggest finding a type name that more closely describes what the methods do. Of course this may prompt additional helper classes, but as long as their names are helpful I don't mind the numbers.

Some time ago I came across a class called XmlHelper during a code review. It had a number of methods that obviously all had to do with Xml. However, it wasn't clear from the type name what the methods had in common (aside from being Xml-related). It turned out that some of the methods were formatting Xml and others were parsing Xml. So IMO the class should have been split in two or more parts with more specific names.

Brian Rasmussen