When does it make sense to use private static methods in instance class

Question

Ive inherited some code that are a regular class with some private static methods in them. The code (pseudo code) looks like this

public class Animal
{
    private string typeOfAnimal;

    public Animal(string typeOfAnimal)
    {
        this.typeOfAnimal = typeOfAnimal;
    }

    public void MakeSound()
    {
        var sound = Animal.GetSound(typeOfAnimal);

        // Make use of sound here       
    }

    private static string GetSound(string typeOfAnimal)
    {
        if(typeOfAnimal  == "dog")
            return "bark";
        else if(typeOfAnimal == "cat")
            return "mjau";
    }
}

Is there any benefit in doing like this compared to making GetSound a regular instance method?

Adam Houldsworth · Answer 1 · 2015-10-19T12:49:37.077

There is some very minor performance difference in static methods, I think that is actually something the SO guys take advantage of. Also, making the method static gets you a slight readability improvement because of what the keyword implies.

My take on this is usually readability. In this case there are two differences: instances vs static, public vs private. Neither is inherently more beneficial than the other, the benefits only appear depending on intended use. In your case, it has no value being a public method and isn't part of the public API of the type so you make it private, and doesn't want to mutate instance state so you make it static.

By default, ReSharper highlights methods that can be made static.

One thing static methods can do that instance methods can't is be called to supply parameters to a base class constructor or forwarded constructors. — Kyle, Oct 19 '15 at 13:37

score 1 · Answer 2 · answered Oct 19 '15 at 12:38

It is advisable to mark your private methods as static if they are not using any of the instance object for slightly better performance and readability.

Infact the following warning in code analysis is shown if such methods are not marked as private.

CA1822: Mark members as static

Extract from the link -

Members that do not access instance data or call instance methods can be marked as static (Shared in Visual Basic). After you mark the methods as static, the compiler will emit nonvirtual call sites to these members. Emitting nonvirtual call sites will prevent a check at runtime for each call that makes sure that the current object pointer is non-null. This can achieve a measurable performance gain for performance-sensitive code. In some cases, the failure to access the current object instance represents a correctness issue.

score 0 · Answer 3 · answered Oct 19 '15 at 13:03

it looks like a bad design. getSound should not be static but implemented in each inherited class.

use static methods when there is no relation between the instance state to the action itself.

in this case, there is a relation. the instance state (type) is done at runtime

i'd write:

public abstract class Animal {
    public abstract string GetSound();
}

public class Dog:Animal{
    public string GetSound(){return "bark";}
}

public class Cat:Animal{
    public string GetSound(){return "mjau";}
}

I agree with this. If Animal is not going to be instantiated then mark it abstract, if there is a chance it could be then you can mark the methods as virtual and then override them in the Derived classes — MattE, Mar 01 '19 at 20:30

When does it make sense to use private static methods in instance class

3 Answers3