Is Resharper's recommendation to make my private method static a good recommendation?

Question

I've recently noticed that when I create private methods that set a few fields in the objects passed to them that Resharper puts up a hint stating that the method can be made static.

Here's a greatly simplified example of the sort of method I might have.

private void MakeStatusTheSame(MyClass mc, MySecondClass msc)
{
    mc.Status = msc.Status;
}

When I've got a method like this, Resharper provides a recommendation that the method can be made static.

I try to avoid making public methods static since they wreck havoc on unit tests...but I'm not sure that the same applies for private methods.

Is Resharper's recommendation a valid best practice or should I just turn it off?

Answer 1

I think that's definitely a prime candidate for a static method. It's not changing any of the class's properties, fields, etc.

Here's an example:

class MyClass
{
  public static void MakeStatusTheSame(MyClass mc, MySecondClass msc)
  {
     mc.status = msc.status;
  }

  private void MakeStatusTheSame(MySecondClass msc)
  {
    this.status = msc.status;
  }

  private int status;
}

Also, you could make it an extension method (which would also be static):

public static class Extensions
{
  public static MyClass MakeStatusTheSame(this MyClass mc, MySecondClass msc)
  {
    mc.status = msc.status
    return mc; /* make the method chainable */
   }
}

Answer 2

I think so; seeing that a method is static is a clear indication that the method should not be interacting with any instance members.

Imagine debugging a non-static method and realizing the instance isn't being touched. Instant smell, and if there isn't a comment explaining what the function does, you could be distracted from the real problem.

Answer 3

I usually go with R#'s recommendation. It's a private method, so (hopefully) you're not writing unit tests against it. Making it static explicitly states that it doesn't use any instance members, which makes it easier to inspect for side effects.

Answer 4

At the risk of sounding like a contrarian, I've got to admit that I don't like mixing static methods with instance methods; and I dislike static methods in general. Static methods are difficult to test, difficult to override, and difficult to maintain. I prefer to stick all static methods for dealing with Foo objects into a single FooUtils class -- or, better yet, into a singleton instance of a FooSomethingDoer class.

Of course, static methods make perfect sense in some cases -- for example, when creating the aforementioned singletons, or factories, etc. I'm not saying that all static methods are made of pure evil; I just prefer to err on the side of avoiding them when possible.

Answer 5

What do you think about code fragmentation in such case? Where run-time compiler creates code for static and non static private members? and When? I think static will be placed in different segment and probably will be compiled when object is not even instantiated. So it can lead to code fragmentation(slow down performance of big applications in low memory case) and bigger application first load time, more memory usage in some cases. Can someone describe low level differences in such case?