Don’t Use “Static” in C#?

I submitted an application I wrote to some other architects for code review. One of them almost immediately wrote me back and said “Don’t use static. You can’t write automated tests with static classes and methods. static is to be avoided.”

I checked and fully 1/4 of my classes are marked static. I use static when I am not going to create an instance of a class because the class is a single global class used throughout the code.

He went on to mention something involving mocking, IOC/DI techniques that can’t be used with static code. He says it is unfortunate when 3rd party libraries are static because of their un-testability.

Is this other architect correct?

update: here is an example:

APIManager – this class keeps dictionaries of 3rd party APIs I am calling along with the next allowed time. It enforces API usage limits that a lot of 3rd parties have in their terms of service. I use it anywhere I am calling a 3rd party service by calling Thread.Sleep(APIManager.GetWait("ProviderXYZ")); before making the call. Everything in here is thread safe and it works great with the TPL in C#.

It depends on whether the static classes maintain state or not. Personally, I have no problem with stateless functions thrown together in a static class.

He is too general about it. He is correct, it hinders testing. However, static classes and methods have their place and it really depends on the context. Without code examples you can’t really tell.

I use static when I am not going to create an instance of a class because the class is a single global class used throughout the code.

This can be severe code smell. What are you using the classes for? To hold data? To access a database? Then he is correct. You should look in to dependency injection in that case, as such a static class is effectively an implicit singleton.

If you are using them for extension methods or helpers that don’t change the state and just operate on the parameters you provide, those usually are fine.

I checked and fully 1/4 of my classes are marked “static”. I use
static when I am not going to create an instance of a class because
the class is a single global class used throughout the code.

The best thing to do is to try and unit-test your code. Try designing tests that are repeatable, independent, simple and test only one method at a time. Try running your tests in different randomised order. Can you get stable “green” build?

If yes, that’s a valid point to defend your code. If, however, you have difficulties, then maybe you should go for instance-based methods.

The answers already posted cover a lot of really good points, but there is one that seems to be missing:

Static fields are never garbage collected.

This is really important if you have an app with a lot of memory constraints, and this pattern can be very common when people try to implement a cache.

Static functions are not nearly as bad, but everyone else has covered this in enough detail already.

One of the advantages that you get from IoC/DI is that most interactions between classes are negotiated between interfaces. This makes unit testing easy because the interfaces can be mocked automatically or semi-automatically, and therefore each of the parts can be tested for inputs and outputs. Furthermore, beyond testabilty, putting interfaces between everything allows you to have the most modular code possible — you can easily replace one implementation without quite so much worry that you’re screwing up dependencies.

Because C# doesn’t have metaclasses, a class can’t implement an interface using static features, so static features of classes end screw up any effort to implement a pure IoC/DI object model. That is to say, you can’t create a mock to test them, and you have to create real dependencies.

If your company/project is invested heavily in doing IoC, this of course is a reasonable concern, and the pain you’re going through is for the gain of all. From an architectural standpoint, however, I personally don’t think that any model should be followed to the grave. There are some things that make sense to implement using static methods — the singleton design strategy, for instance. I’m less inclined towards static classes myself because I think they tend to start to lose the advantages of OO, but there are times when a library class is probably a more natural expression of something than an engine.

Commenter reminds me of extension methods, which of course have to be in static classes in C#. That’s legit, and is an example of how pure IoC doesn’t work super well in C#, at least if you’re trying to take advantage of the breadth of the language.

Static methods are fine to use and have a rightful place in programming. It sounds like your architect has a testing framework that does not fully support static methods. If your code is part of a bigger project, then it is important to meet the architects guidelines. However, don’t let this project deter you from using static methods when it is appropriate to do so.

Static classes are sometimes misused and should be:

a singleton (where the non-static class has a static member and a public static Instance variable to retrieve/create it only once.
a method in another class (where the state matters). If you have a member that is using no data from that class, probably it should not be part of that class.

It also might be a so called ‘utility’ function, and part of a utility class. Utility classes are classes that contain only static methods and serve as helper functions without any context.

I have been using static properties for things that are common to all instances of a class. And I have been using static methods to get groups of class objects. I am by no means an expert but this has been working for me so far.

PHP example:

class vendorData {
  private static $data_table = 'vendor_data'; // static property
  private $id; // instance property
  private $name; // instance property

  public function __construct($vendor_id) {
    if(!self::validId($vendor_id) ) { // static method
      return false; // id doesn't exist
    }
    $this->id = $vendor_id; // id has been validated
    $this->getProperties(); // object method
  }

  private static function validId($vendor_id) {
    // send db query to find if the id exists
    // return true/false;
  }

  private function getProperties() { // object method
    $sql = "SELECT * FROM `{self::$data_table}` // using static property
        WHERE `id` = {$this->id}"; // using object instance property
    // get resultset
    foreach($result as $property => $value) {
      $this->$property = $value; // object instance properties all set
    }
  }

  // and here
  public static function getBy($property,$value) { // static method to return object array
    $sql = "SELECT `id` FROM `{self::$data_table}` // using static property
      WHERE `$property` = '$value'";
    // send query, get $ids array
    $obj_array = array();
    foreach($ids as $id) {
      // create array of current class objects using special static keyword
      // meaning of static here is different than in property/method declarations
      $obj_array[$id] = new static($id);
    }
    return $obj_array;
  }
}

I’ll say the principles they have are ok but the statement (don’t use statics) may be wrong. How much is your code coverage? if the number is high and you are comfortable with the unit testing of your app, then you are ok. If not, then I would suggest review the code. You may find static classes are the reason or not, it will depend on a lot of things.

Classes with static methods abuse the principles of OOP. It’s not an OOP already, it’s COP — class-oriented programming.

They rarely have clear “personality” (I use my favorite human metaphor here), or identity. So they don’t know who they are. This point alone is enough to get rid of this concept in OOP, but there are more of them.

The next one is a consequence of the first point. Since such classes don’t know who they are, they tend to be from big to huge.

Third one makes your code absolutely non-maintainable. The use of static methods introduces hidden dependencies. They pop up all over the place, and you never know what’s happening in your class. You can’t be sure in that just by looking at the constructor signature.

Slowly, but inevitably, your code’s getting less and less cohesive, since hidden dependencies are poorly controlled. They seem invisible. And it’s so easy to add another one! You don’t have to modify any method’s signature. All you have to do is to call a static method. And what an ugly code follows…

Ok, you probably already guessed. Tight coupling is an oft attendant of low cohesion. If there are a lot of clients using some class, then the coupling is getting tighter. And there is a big seduction in using an already written class or method that requires only tiny fix. Only one method flag-parameter.

What to use instead of static methods? Well, my suggestion is OOP. Why not giving it a try?

Filed under: softwareengineering - @ 10:51

Thẻ: architecture, c++, class, static-keyword

Thiết kế website giá rẻ

Danh mục

Don’t Use “Static” in C#?