Is it bad practice to have state in a static class?

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) {
        retur开发者_开发知识库n 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.

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.

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.

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

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;
        }
    }

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.

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.

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?