Killing the Helper class, part two

[アーティクル]
09/07/2005

Earlier this week, I blogged on the evils of helper classes. I got a few very thoughful responses, and I wanted to try to address one of them. It is far easier to do that with a new entry that trying to respond in the messages.

If you didn't read the original post, I evaluated the concept of the helper class from the standpoint of one set of good principles for Object Oriented development, as described by Robert Martin, a well respected author and speaker. While I don't claim that his description of OO principles is "the only valid description as annointed by Prophet Bob", I find it extremely useful and one of the more lucid descriptions of fundamental OO principles available on the web. That's my caveat.

The response I wanted to address is from William Sullivan, and reads as follows:

I can think of one case where helper classes are useful... Code re-use in a company. For instance, a company has a policy on how its programs will access and write to the registry. You wouldn't want some products in the company saving its data in HKLM/Software/CompanyName/ProductName and some under .../Software/ProductName and some .../"Company Name"/"Product Name". So you create a "helper class" that has static functions for accessing data in the registry. It could be designed to be instantiatable and extendable, but what would be the advantage? Another class could implement the companies' policy on encryption, another for database access;[clip]

If you recall, my definition of a helper class is one in which all of the methods are static. It is essentially a set of procedures that are "outside" the object structure of the application or the framework. My objections were that classes of this nature violate two of the principles: the Open Closed Principle, and the Dependency Injection Principle.

So, let's look at what a company can do to create a set of routines like William describes.

Let's say that a company (Fabrikam) produces a dozen software systems. One of them, for our example, is called "Enlighten". So the standard location for accessing data in the registry would be under HKLM/Software/Fabrikam/Enlighten. Let's look at two approaches: one using a helper class and one using an instantiated object:

class HSettings
{
   static String GetKey(String ProductName, String Subkey)
   {   // -- interesting code
   }
}

class FSettings
{
     private string _ProductName;
     public FSettings (String ProductName)
     {   _ProductName = ProductName;
     }
     public String GetKey(String Subkey)
     { // nearly identical code
     }
}

Calling the FSettings object may look to be a little more effort:

public String MyMethod()
{   FSettings fs = new FSettings("Enlighten");
    string Settingvalue = fs.GetKey("Mysetting");
    //Interesting code using Settingvalue
}

as compared to:

public String MyMethod()
{ string Settingvalue = HSettings.GetKey("Enlighten","Mysetting");
//Interesting code using Settingvalue
}

The problem comes in unit testing. How do you test the method "MyMethod" in such a way that you can find defects in the 'Interesting Code' without also relying on any frailties of the code buried in our settings object. Also, how to test this code without there being any setting at all in the registry? Can we test on the build machine? This is a common problem with unit testing. How to test the UNIT of functionality without also testing any underlying dependencies.

When a function depends on another function, you cannot easily find out where a defect is causing you a problem. A defect in the dependency can cause a defect in the relying code. Even the most defensive programming won't do much good if the dependent code returns garbage data.

If you use the Dependency Injection Principle, you can get code that is a lot less frail, and it easily testable. Let's refactor our "FSettings" object to inherit from an interface. (This is not something we can do for the HSettings class, because it is a helper class).

Interface ISettings
{
public String GetKey(String Subkey);

}
class FSettings : ISettings // and so on

Now, we refactor our calling code to use Dependency Injection:

public class MyStuff {
private ISettings _fs; public MyStuff() {
    _fs = new FSettings("Enlighten");
}
public SetSettingsObject(ISettings ifs)
{
    _fs = ifs;
}
public String MyMethod()
{    string Settingvalue = _fs.GetKey("Mysetting");
    //Interesting code using Settingvalue
}
}

Take note: the code in MyMethod now looks almost identical to the code that we proposed for using the static methods. The difference is important, though. First off, we seperate the creation of the dependency from it's use by moving the creation into the constructor. Secondly, we provide a mechanism to override the dependent object.

In practical terms, the code that calls MyMethod won't care. It still has to create a 'MyStuff' object and call the MyMethod method. No parameters changed. The interface is entirely consistent. However, if we want to unit test the MyMethod object, we now have a powerful tool: the mock object.

class MockSettings : ISettings
{
     public MockSettings (String ProductName)
     {   if (ProductName != "Enlighten")
        throw new ApplicationException("invalid product name");
     }
     public String GetKey(String Subkey)
     { return "ValidConnectionString";
     }
}

So, our normal code remains the same, but when it comes time to TEST our MyMethod method, we write a test fixture (a method in a special class that does nothing but test the method). In the test fixture, we use the mock object:

class MyTestFixture
{
     public void Test1 ()
     {   MyStuff ms = new MyStuff();
         MockSettings mock = new MockSettings();
         ms.SetSettingsObject(mock);
            // now the code will use the mock, not the real one.
         ms.MyMethod();
        // call the method... any exceptions?

      }
}

What's special about a test fixture? If you are using NUnit or Visual Studio's unit testing framework, then any exceptions are caught for you.

This powerful technique is only possible because I did not use a static helper class when I wanted to look up the settings in my registry.

Of course, not everyone will write code using unit testing. That doesn't change the fact that it is good practice to seperate the construction of an object from it's use. (See Scott Bain's article on Use vs. Creation in OO Design). It also doesn't change the fact that this useful construction, simple to do if you started with a real object, requires far more code change if you had started with a helper class. In fact, if you had started with a helper class, you may be tempted to avoid unit testing altogether.

I don't know about you, but I've come across far too much code that needed to be unit tested, but where adding the unit tests would involve a restructuring of the code. If you do yourself, and the next programmer behind you, a huge favor and simply use a real object from the start, you will earn "programmer's karma" and may inherit some of that well structured code as well. If everyone would simply follow "best practices" (even when you can't see the reason why it's useful in a particular case), then we would be protected from our own folly most of the time.

So, coming back to William's original question: "it could be designed to be instantiable and extendable, but what's the advantage?"

The advantage, is that when it comes time to prove that the calling code works, you have not prevented the use of good testing practices by forcing the developer to use a static helper class, whether he wanted to or not.

Comments

Anonymous
September 07, 2005
I think that one type of useful helper class is to provide a concept of the "current" implementation of an interface to use. Extending your above example, we can avoid the code to inject each similar dependency into each similar client by creating a class like:

public static class Settings
{
public static ISettings Current
{
get
{
// return the current settings manager to use based on configuration
}
}
}

This helper class then allows callsites to simply use Settings.Current.GetKey( ) instead of each maintaining a reference to the current settings which must be injected into each callsite. This sacrifices flexibility (call sites all end up with the same ISettings implementation) but gains simplicity and consistency. I think this is a valid use of helper classes (possibly the valid use, except maybe something like System.Math).
Anonymous
September 07, 2005
These are all very good points.

I'd like to give what I think is a counterexample, mainly because I'm curious to see what your opinion is on whether this is one of the rare cases when a helper class is actually good, or whether you see a better way.

In this particular case I think it would be hugely painful to try to use any other approach, but perhaps you can convince me I'm wrong.

The example? System.Web.HttpUtility.
Anonymous
September 07, 2005
Stuart:
I think that it is unfortunate that the HttpUtility class is a helper class in the framework. It is not the only one. I'm sure that the .Net team had long "discussions" over things like this.

It is quite simple to use another approach. Take a look at the URI type (returned by the Response.URL method). It would have been quite simple to add the various 'encode' and 'decode' methods to a URL object that would be either similar to the URI object or inherited from it.

The wierd thing is that there IS a URL type, but it is only used for permissions for assemblies, AFAIK.

I disagree that this is a counterexample. A well implemented URL object would have no problem offering the ability to URLEncode and URLDecode. A well implemented HttpContent object would have no difficulty providing an HTTPEncode and HTTPDecode method. These were choices made by others.

Note that I'm not saying that all helper classes are to be banned for good. I am saying that they are inherently bad and that the developer who uses them should understand that fact.
Anonymous
September 07, 2005
Douglas,

You've effectively created a Singleton. Singletons that are created for global access to a class are simply the OO equivalent of a global variable.

Global variables lead to a common anti-pattern called "The Big Ball of Mud" (I'm not kidding).

This does make the code a little simpler to write. However, Dependency Injection is NOT a problem. It is a best practice. Inserting an antipattern because you want to avoid a best practice seems odd to me.

Respectfully,
--- Nick
Anonymous
September 08, 2005
Actually UrlEncode and UrlDecode would not be appropriate as instance methods on an Url or Uri object, because they're frequently used on small fragments of strings that will later be combined into an actual Url. In other words it's very common to want to call them when you don't actually have an url around at all. Static methods on an Url class, perhaps, but then what does that buy you over a helper class except for violating the separation of responsibilities because Url is then responsible for being an Url and for providing static helper methods. Seems worse to me.

Perhaps a more object-oriented approach would be as something like:

class UrlFragment {
static UrlFragment FromEncodedString(string s);
static UrlFragment FromDecodedString(string s);
string Encoded {get;}
string Decoded {get;}
}

Then you could translate HttpUtility.UrlEncode(str) as UrlFragment.FromDecodedString(s).Encoded. But again, I don't see what that really buys you except more code to type - it's not clear how you'd substitute in an alternative implementation of UrlFragment or, for that matter, why you'd want to.

As far as Douglas's example, I think the idea would be that the Singleton could also have a setter on the Current property for use in testing. That way all the test code could just replace that object in one place. Seems like a good idea to me, and Settings is still a helper class by your definition.
Anonymous
September 08, 2005
The problem is that you do not necessarily want the URLEncode method to only be on a URL object. URL encoding is effectively text processing, and contrary to the name, you are not always operating on a full URL when you call that function.

In a project that I have worked on, keywords are stored as URL encoded text because they get encoded into a URL on an outside system. This is unfortunate that this external system imposed this requirement, but that does not change my life at all. Thus, when we store keywords in the repository, we need to URL encode them without ever manipulating them as a URL. If the only way that the URLEncode method was exposed was on a URL object. I would be forced to create a fake URL (e.g. http://example.net/) to trick the URL object into accepting it. That would be bad.

I appeciate this settings example and I have used that pattern many times. Though one difference between a helper class that does preferences, and a helper class that does in memory text processing is that a settings class interacts with outside systems (such as the registry). Something like URL encoding is mere text processing, and if you are going to unit test with mock URL encoding, you may as well unit test with mock strings or mock sine and cosine code. That is definately going overboard.
Anonymous
September 08, 2005
It is interesting that both Stuart and Nate pointed out that URL Encoding is not necessarily something that you would just do to a URL, but in fact to a string.

What does that tell me? That URL Encoding is a method that should be added to an object that inherits from the string object.

Unfortunately, the String object is sealed. I consider this to be a difficult decision at best on the part of the framework developers, and I do not understand it. Some folks have explained this as a code safety issue, others as CLR optimizations... I don't have the advantage of ScottGu's ear, so I cannot speculate. I can say, however, that from an OO principle standpoint, all you have succeeded in doing is convincing me that URLEncode is not a method of a URL, but rather a method of an extended string object.

So, we have a practical reason for using a helper class (the sealed nature of string objects) but not a principled one.

Unless or until the base string type becomes an unsealed object, we are stuck with these helper classes.

Stuart: The singleton with a setter is an interesting design. It is not as bad as a pure helper class. You still have to create a type that can be instantiated in order to store it in the singleton. You can extend this object and you still maintain dependency injection. Yes, you are limited in your ability to extend the singleton wrapper, but that is a small price to pay. On this basis, it would appear to address my concerns. My problem is the introduction of the Singleton as a global variable.

I will blog seperately about the evils of using a Singleton as a back-door way to introduce a global variable. At this time, suffice it to say that I don't find global variables to be an appealing alternative to static helper classes.
Anonymous
September 09, 2005
Is it really kosher OOP to subclass for the sole purpose of aggregating additional functionality without modifying nor customizing old behavior?

Suppose for the sake of argument, the string class was not sealed, and we subclassed it to an extended class with URLEncode and URLDecode. Now I have a problem because the strings that the .NET framework return are not the strings I want; it isn't like the XmlTextNode.Text property or the Stream.ReadLine method will all of a sudden return URLEncodableString instead of System.String.
Anonymous
September 10, 2005
I came across a couple of good posts here and here on helper classes.
Although I don't believe helper...
Anonymous
August 21, 2006
I never like too much the concept of "helper classes", &nbsp; First of all I tried to found some written...
Anonymous
September 08, 2007
PingBack from http://mikesdump.wordpress.com/2005/09/10/the-evils-of-helper-classes/
Anonymous
February 09, 2014
Pingback from Type Safe C# « Non Geek for the Geeks
Anonymous
February 09, 2014
Pingback from Type Safe C# « Non Geek for the Geeks
Anonymous
February 09, 2014
Pingback from Type Safe C# « Non Geek for the Geeks
Anonymous
February 09, 2014
Pingback from Type Safe C# « Non Geek for the Geeks
Anonymous
February 20, 2014
Pingback from Type Safe C# « Non Geek for the Geeks
Anonymous
November 17, 2014
Pingback from When we combined my supporter classes, am we over designing? | Zerbe

次の方法で共有

Killing the Helper class, part two

Comments

その他のリソース