Opinion on Making a Common Utility Class Static

Question

Some of what I've been reading about this yesterday and today:

• (SO) Should you avoid static classes?
• (SO) When to Use Static Classes in C# - Mark S. Rasmussen makes a nice comment here
• Singleton vs Static - Nice quote from here:

  These differences are extremely subtle but they do impact how resilient the system will hold up after years of subtle changes, new features, enhancements, etc. Most developers dislike system maintenance and I feel the biggest reason is because systems aren't really designed to be maintained. By incorporating the "singleton" pattern above, I've abstracted object creation and typing away from my core business logic which is critical to long-term maintenance.

• Singleton Considered Stupid
• (SO) Class with single method — best approach? - Once again, a quote from Mark S. Rasmussen:

  Demanding consumers to create an instance of classes for no reason; True utility classes that do not pose any risk to bloat are excellent cases for static methods - System.Convert as an example. If your project is a one-off with no requirements for future maintenance, the overall architecture really isn't very important - static or non static, doesn't really matter - development speed does, however.

• (SO) Making Methods All Static in Class - Speed is not an issue right now for me, the code must work *PROPERLY* when called by multiple clients from the website and from (possibly, but unlikely) multiple clients from the admin app.

We are (I am) busy rewriting of of our main applications at the moment, and one of the projects in the solution is "Common" and it contains a single class, "clsCommon". This class has no properties, a single private method that is called as the only line in the constructor and then just public methods.

This class handles things like (not the real method names or signatures):

• Copying files:
  • CopyFile
  • DecryptAndCopyFile
• XML Settings File:
  • GetSpecificXMLValue (queries an XML settings file for a value)
  • SetSpecificXMLValue
  • DeleteSpecificXMLValue
• Creating directories
• Getting timestamps as strings (`return DateTime.Now.ToString("yyyyMMddHHmmss");`)
• Various string functions on passed parameters
• Writing out to log files.
• Checking a passed string parameter if it contains only elements in a whitelist (the whitelist is `readonly` private List that is assigned a value in the constructor.)

Currently all the other projects in the solution have a reference to this project, and they provide access to the class by:

namespace Us.OtherProjects
{
   public class clsCommon : Us.Common.clsCommon
   {}
}

namespace Us.OtherProjects
{
   public static class clsMain
   {
         public static clsCommon CommonClsClassReferenceForGlobalUsage = new clsCommon();
         .
         .
         .
   }
}

namespace Us.OtherProjects
{
   public class clsOtherClass
   {
      .
      .
      .
         private void someMethod()
         {
            string something = clsMain.CommonClsClassReferenceForGlobalUsage.Foo();
         }
      .
      .
      .
   }
}

What are your opinions about making this class a static class, or maybe a singleton as described by Jon Skeet here?

Answer 1

When i hear words Common, Manager, Helper I instantly think about code smell. There is nothing easier than naming class w/o actual meaning and just burrow huge plumbing code inside. Usually this happens when developer is too influenced with procedural programming.

Answering to question - I dislike static things in general (concurrency problems, harder instance life time management etc.). There are rare cases when static is useful and i believe this ain't one of these.

---

If 'wannabe static helper method' fits, write extension method. If it doesn't, then it should be in separate service or in base class of something (that depends on context).

---

So a class that manages things shouldn't be called SomethingManager? I disagree in that specific case

There are cases when it might be appropriate. But as i see it - 'XManager' just tells you that this 'Manager' class works with class 'X' and 'manages' something whatever 'managing' is supposed to mean. Helpers help with something. What exactly? Who knows - got to check out code. 'Common' is even more vague. Have seen every kind of stuff in such a classes/namespaces/projects (data access, security and UI related stuff tied together with business domain logic).

---

Hmmm... so maybe namespace Us.Common { public (static?) class Strings {} public (static?) class Files {} public (static?) class Settings {} } Opinions on this architecture change? Once again, I'm interested in maintainability and usability please.

You might find useful this post. According to it - it's Logical cohesion in Your case.

Answer 2

The criterion here is state. If your class is just a container for independent functions (ie CopyFile, GetTimeStamp) a static class is the most logical approach. See the System.Math class.

However your class has a constructor and i assume it holds some state (name of config/log file). That calls for a Singleton.

Looking at the list of functionality, I think you should refactor this into several classes. Some can (and should) be static. Use Singleton(s) for the parts that need state, like Logging and Configuration etc.

The main design issue I see here is having 1 class that does Logging, Configuration, Formatting and a bunch of other stuff as well.