CODEWITHSUNDEEP
c#staticstatic-methodsstatic-members
Shruti Sharma
Shruti Sharma

Reputation: 103

Using Static method and variables - Good vs Bad

I am developing C# and asp.net web application.

I have general class called utilities, I have lot of public and static variables in this public utilities class.

Since this number is gradually increasing, I want to know is it good practice to store utilities methods and variable as public static.

Example of my code

public class utilities
{
    public static string utilVariable1 = "Myvalue";
    public static string utilVariable2 = "Myvalue";
    public static string utilVariable3 = "Myvalue";
    :
    public static string utilVariableN = "Myvalue";


    public static string UtilMethod1()
    {
         //do something
    }

    public static string UtilMethod2()
    {
         //do something
    }

    public static string UtilMethodN()
    {
         //do something
    }
}

Upvotes: 10

Views: 13781

Answers (5)

Antony Woods
Antony Woods

Reputation: 4462

There's nothing wrong with this approach for methods, but variables should really be const if they're going to be static and public. If they are subject to change then you should look at a different structure for variables that are being manipulated by more than one component.

Personally, I'm a fan of the Singleton pattern.

Upvotes: 6

Aidiakapi
Aidiakapi

Reputation: 6249

Creating one class to do it all is not a good practice, and it's recommended to structure your project, and keep stuff that belongs to each other separated from the randomness.

A great example of this was a project I took over from a co-worker. There was 1 class, called Methods. It contained over 10K lines of methods.
I then categorized them into approx. 20 files, and the structure was restored.

Most of the methods from that project were validating user input, which can easily be moved into a static class Validation.

One awful thing I notice is the mutable public and static variables. This is bad for several reasons:

  1. Incorrect behavior, because if some method changes this, while it isn't supposed to do that, it causes other methods to behave improperly, and it's really hard to track down/debug.
  2. Concurrency, how are we going to ensure thread safety? Do we let it over to all methods that work with that? Say if it's a value type, what will we let them lock on? What if some method forgets to make it thread safe?
  3. Expand-ability (I hope you understand what I mean with that), if you have for example a static class data that stores all these public static variables, that you shouldn't have. It can store that once, if for example you might change your application structure a bit, and say want to make it possible to load two projects in the same screen, then it's very difficult to make that possible, because you can't create two instances of a static class. There is only one class, and it'll remain like that.

For number 3 a cleaner solution would be to store either a list of instances of a data class, or to store a reference to the default and/or active data class.

Static member, and private static members (or protected) are a good practice, as long as you don't make huge classes, and the methods are related.

Public and static variables are okay if they're not really variable.
The two ways to do this is by marking them constant (const modifier) or readonly (readonly modifier).

Example:

public class UtilitiesClass
{
    internal UtilitiesClass() { }

    public void UtilityMethod1()
    {
        // Do something
    }
}

// Method 1 (readonly):
public static readonly UtilitiesClass Utilities = new UtilitiesClass();

// Method 2 (property):
private static UtilitiesClass _utilities = new UtilitiesClass();
public static UtilitiesClass Utilities
{
    get { return _utilities; }
    private set { _utilities = value; }
}

The advantage of method 1 is that you don't have to worry about thread-safety at all, the value can't change.
Method 2 is not thread-safe (though it's not difficult to make it that), but it has the advantage of allowing the static class itself to change the reference to the utilities class.

Upvotes: 2

Daniel B
Daniel B

Reputation: 2887

There's nothing inherently wrong with static classes, although they should typically not have state (fields). Your use of public static fields indicates that this is not the case, so it seems like you are using abusing the static keyword slightly. If your class needs to have state, then it should be a normal, non-static class, and you should create instances of it. Otherwise, the only public fields visible on the class should be const (consider the Math class, with constants such as Math.PI - a good use of static methods and fields).

Another consideration is cohesion. Methods typically exist grouped in one class because they are closely related in one way or another. Again, the Math class is a good example; everything in there has to do with maths. At some point, you would want to split your global utility class into multiple smaller, more focussed ones. See Wikipedia for some examples on cohesion, it sounds like your usage falls under "Coincidental cohesion (worst)".

Upvotes: 13

thumbmunkeys
thumbmunkeys

Reputation: 20764

static is not a bad thing per se. Methods that don't need to access any member variables or methods should always be declared static. That way the reader of the code sees immediately that a method won't change member variables or methods.

For variables the situation is different, you should avoid static variables unless you make them const. Public static variables are globally accessible and can easily raise issues if multiple threads access the same variable without proper synchronization.

It is hard to tell for your case if it's a good or a bad idea to use statics, because you didn't provide any context information.

Upvotes: 3

Jonas Høgh
Jonas Høgh

Reputation: 10874

No, it is not a good practice for large applications, especially not if your static variables are mutable, as they are then effectively global variables, a code smell which Object Oriented Programming was supposed to "solve".

At the very least start by grouping your methods into smaller classes with associated functionality - the Util name indicates nothing about the purpose of your methods and smells of an incoherent class in itself.

Second, you should always consider if a method is better implemented as a (non-static) method on the same object where the data that is passed as argument(s) to the method lives.

Finally, if your application is quite large and/or complex, you can consider solutions such as an Inversion of Control container, which can reduce the dependency on global state. However, ASP.Net webforms is notoriously hard to integrate into such an environment, as the framework is very tightly coupled in itself.

Upvotes: 1

Related Questions