ASP.NET and static method in controller

Question

Assume that WebApi2 controller has a SearchClient which is configured in scoped-lifestyle dependency on startup.

public class SearchController : ApiController {

    private readonly SearchClient _indexClient;

    public SearchController(SearchClient client) {
        _indexClient = client; // dependency injected
    }

    public IEnumerable<string> Get(string keyword){
        return SearchDocuments(_indexClient, keyword);
    }

    public static IEnumerable<string> SearchDocuments(SearchClient indexClient, string text)
    {
        return indexClient.Search(text);
    }
}

As we can see, SearchDocuments method has static keyword.

My questions are;

How can we decide whether static method is good or bad?
Is static method safe or recommended in such a multiple-accessed web environment?
What about async static method in web environment? Is it different from async method?

Why would you make it static and require a SearchClient when it could be instance and not need the parameter? But, static is not safe for properties that are changed by controller actions, not for methods or actually static properties (that don't change). — Camilo Terevinto, Jul 12 '17 at 11:10
That static method serves no real purpose. On a separate not the controller should depend on abstractions and not concretions, assuming `SearchClient` is not an abstract class. — Nkosi, Jul 12 '17 at 11:11
@CamiloTerevinto // Well, sometimes, Resharper recommends to make it. And it allows to move method to another class less risky which makes refactoring easier. — Youngjae, Jul 12 '17 at 11:12
Resharper is recommending to move an instance method (cause it uses instance data) to a static method? That's weird. I do agree it's easier to move out of the class though — Camilo Terevinto, Jul 12 '17 at 11:14
@CamiloTerevinto // yes, Resharper occationally recommends it. similar discussion is here; https://stackoverflow.com/q/169378/361100 — Youngjae, Jul 15 '17 at 10:56

score 6 · Accepted Answer · edited Jul 12 '17 at 11:13

How can we decide whether static method is good or bad?

Static methods in web applications are just like static methods in desktop applications. There is no difference in how they are handled or interpreted once they run in a web application. So they are not bad or good, you use them for everything that is not instance-specific.

Is static method safe or recommended in such a multiple-accessed web environment?

static fields or properties can have unwanted side effects when user or session specific data is stored in it since static variables are shared across sessions. But this is a method, and methods don't have a shared state. Hence they are safe to use in a multi-user/multi-session environment.

What about async static method in web environment? Is it different from async method?

Nothing other than already described in the answer to your first question.

I'm glad I have the same understanding a 101k user has. +1 to you for confirming my small comment — Camilo Terevinto, Jul 12 '17 at 11:16
I could have had just 100 reputation if I didn't spend so much time here... Reputation says not so much. @CamiloTerevinto — Patrick Hofman, Jul 12 '17 at 11:17

Nkosi · Answer 2 · 2017-07-12T11:26:29.140

Just to add to the already provided answers.

The use of static method on the controller does not really add any value and is not actually needed in the given scenario.

Consider abstracting explicit dependencies in the controller.

public class SearchController : ApiController {

    private readonly ISearchClient indexClient;

    public SearchController(ISearchClient client) {
        indexClient = client; // dependency injected
    }

    public IEnumerable<string> Get(string keyword){
        return indexClient.Search(keyword);
    }
}

This will also allow for loose coupling and more flexibility when testing and also refactoring as the implementation of the dependency can change without having to touch the controller.

ASP.NET and static method in controller

2 Answers2