views:

254

answers:

7

I would like to do something like this:

public class Foo {
    // Probably really a Guid, but I'm using a string here for simplicity's sake.
    string Id { get; set; }

    int Data { get; set; }

    public Foo (int data) {
        ...
    }

    ...
}

public static class FooManager {
    Dictionary<string, Foo> foos = new Dictionary<string, Foo> ();

    public static Foo Get (string id) {
        return foos [id];
    }

    public static Foo Add (int data) {
        Foo foo = new Foo (data);
        foos.Add (foo.Id, foo);

        return foo;
    }

    public static bool Remove (string id) {
        return foos.Remove (id);
    }

    ...

    // Other members, perhaps events for when Foos are added or removed, etc.
}

This would allow me to manage the global collection of Foos from anywhere. However, I've been told that static classes should always be stateless--you shouldn't use them to store global data. Global data in general seems to be frowned upon. If I shouldn't use a static class, what is the right way to approach this problem?

Note: I did find a similar question, but the answer given doesn't really apply in my case.

+3  A: 

What you seem to be looking for here is a singleton class, not a static class. Static classes and methods should be referred for stateless routines. A singleton class gets instantiated once and only once per application run and has the full functionality of a class as such. Every time that you reference it in the future, you will get back the exact same instance with the exact same member properties.

The first google result for "C# singleton" seems to have a pretty decent explanation for implementation. http://www.yoda.arachsys.com/csharp/singleton.html

phantomdata
be aware though that singletons aren't that good practise either, because they create dependencies etc. In the end, IoC (Inversion of Control) and Dependency Injection would be a better solution.
Femaref
+3  A: 

Global data is both powerful and a common source of problem, that's why techniques like dependency injection are used. You can think of it as a normal decoupling problem. Having global data that is referenced directly in a lot of places in your program makes a strong coupling between that global data and all those places.

In your example however you have isolated the access to the data into a class, which controls the exact details of the access of the global data. As some global data is often inevitable, I think that this is a good approach.

You can for example compare to how app.config and web.config are used through the .NET framework. They are accessed through a static class System.Configuration.ConfigurationManager with a static property AppSettings, which hides a away the details of how to reach the global data.

Anders Abel
+2  A: 

It's not generally bad. In some rare cases it's neccessary to do it that way than implementing something else with large overhead.

But it's recommended to watch for threadsafety.

You should lock every call to your dictionary so that only one thread can access it at one time.


private static readonly object LockStaticFields = new object();

public static Foo Add (int data) {
        lock(LockStaticFields)
        {
           Foo foo = new Foo (data);
           foos.Add (foo.Id, foo);

           return foo;
        }
    }

DHN
I would add: "You should lock every call to your dictionary" if you are working in multi-threading environment. Otherwise it will be useless overhead
Budda
Hmm, perhaps you're right. But my experience tells me that it's better to do always that way. I'm working in company which now has to refactor almost everything because initially it was not implemented for multithreading environments. I think this costs much more than implementing it threadsafe the first time although you may not need this safety.
DHN
A: 

Use a read only static property in your class, this property will be the same across all instances of the class. Increment it and decrement as needed from the constructor, etc.

Brettski
+7  A: 

Who stays that static classes should be stateless? Static means stated.

Just know how static classes work in the CLR:

  • You can't control the time when static constructors are called.
  • Static classes have a separate state for each calling program.

Also be aware of concurrency issues.

As a side note, it amazes me how often people say "Don't use X." It would be like someone walking into your toolshed and pointing to half a dozen tools and saying, "Those tools are bad practice." It doesn't make sense.

harpo
Yes, we should all use more global variables and gotos...
Lucas
Use it when you have to, but be smart to avoid issues. Later be agile to move to something better.
Lex Li
A: 

I use lists in static classes all the time for things that will never (or extremely rarely) change - handy for for loading pick lists and such without hitting the db every time. Since I don't allow changes, I don't have to worry about locking/access control.

Ray
A: 

One other thing to take into consideration is the application itself and the budget for it. Is there really a need for something more complex than a static class?

George Handlin