AndrewJacksonZA
AndrewJacksonZA

Reputation: 1273

Opinion on Making a Common Utility Class Static

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

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):

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?

Upvotes: 5

Views: 2597

Answers (2)

Arnis Lapsa
Arnis Lapsa

Reputation: 47587

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.

Upvotes: 6

Henk Holterman
Henk Holterman

Reputation: 273244

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.

Upvotes: 5

Related Questions