Opinion on Making a Common Utility Class Static

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.

+1 Agreed. Ayende provides the ultimate explanation IMO: http://ayende.com/Blog/archive/2009/04/29/let-us-burn-all-those-pesky-util-amp-common-libraries.aspx

Mark Seemann 2010-02-25 09:40:10

So a class that manages things shouldn't be called SomethingManager? I disagree in that specific case. And frankly if you need a small specific operation that you perform in several different places on something, like an XmlDocument, I don't even consider a static helper class bad design as long as it doesn't become a "do everything" class.

Skurmedel 2010-02-25 09:42:56

AndrewJacksonZA 2010-02-25 09:46:50

@AndrewJackson It's hard to answer how exactly you need to structure your code without knowing exact context. This architecture change isn't good enough for me, but it's better than having 'everything related' stuff inside one class.

Arnis L. 2010-02-25 09:55:19

Thanks for your help Arnis.Judging from your post I'm assuming that you can't recommend a better architecture based on the information that I've provided?

AndrewJacksonZA 2010-02-25 10:30:54

Yes and no. And technically - i already did. Architecture just isn't simple enough to be passed around like an e-mail. Re-read answer by Henk and blog post by Ayende - i can't add much more to this topic.

Arnis L. 2010-02-25 10:58:42

Thanks for the link Arnis, interesting reading...

AndrewJacksonZA 2010-03-30 11:28:36

More useful answer than mine. :)

Arnis L. 2010-02-25 09:59:04

The constructor just instantiates and adds the appropriate whitelist characters to the private readonly whiltelist.I'm also leaning towards a singleton, but I just have this feeling... something's bugging me about changing this class (which I know looks like a bucket containing random methods that are called from multiple projects that the original developers - from a VB6 background - might not have had the time to do correctly.) Like I said, this class is from the website code which we still have to rewrite and from the admin app - does this make a...

AndrewJacksonZA 2010-02-25 10:28:26

difference at all? It doesn't appear to, but I'm not sure. Maybe just paranoia? :-)

AndrewJacksonZA 2010-02-25 10:28:58

@Andrew, it does not matter all that much. You probably should follow the "if it is not broken, don't fix it" rule here. It sounds like the constructor could be eliminated entirely, would make thing a little easier. But you still haven' described what state (fields) it contains.

Henk Holterman 2010-02-25 10:37:15

@Henk: private readonly List<char> m_DatabaseAttributeNameWhitelist; private const string THE_SYSTEM_XML_FILENAME = "MyConfigFile.xml"; private readonly string THE_SYSTEM_XML_PATH = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().GetName().CodeBase.ToString()).Replace("file:\\", String.Empty), THE_SYSTEM_XML_FILENAME); private const string THE_SYSTEM_XML_SYSTEM_SECTION_NAME = "System"; private const char DATABASE_ATTRIBUTE_NAME_REPLACEMENT_CHARACTER = '_';

AndrewJacksonZA 2010-02-25 10:50:43

@Henk: Constructor contents: m_DatabaseAttributeNameWhitelist = new List<char>(63); // Add 0 - 9, 'A' - 'Z', 'a' - 'z' and DATABASE_ATTRIBUTE_NAME_REPLACEMENT_CHARACTER to m_DatabaseAttributeNameWhitelist

AndrewJacksonZA 2010-02-25 10:50:48

AAARRRGGGHHH!!! How does one format code in a comment?

AndrewJacksonZA 2010-02-25 10:51:11

@AndrewJackson: Wrap it in accents, like this: ``code``

Skurmedel 2010-02-25 22:11:09

ansaurus

tags:

views:

answers:

Opinion on Making a Common Utility Class Static

related questions