9

I was wondering why resharper suggests a method to be static in non static class? Is it for saving the creation of the instance? Is it a matter of performance? also, why does it suggest to 'const' some params? Is it a matter of performance ? I would love to get some explanation

carlosdc
  • 12,022
  • 4
  • 45
  • 62
Royi Namir
  • 144,742
  • 138
  • 468
  • 792
  • Check out this [answer](http://stackoverflow.com/a/790318/945456) to a very similar [question](http://stackoverflow.com/q/790281/945456) for design-feedback perspective as to why ReSharper does this. – Jeff B Mar 06 '13 at 20:58

6 Answers6

26

When compiler encounters a static method it emits call instructions but when it encounters an instance method it emits callvirt instruction. Now the callvirt instruction checks the object whether it is null before making the call. so there is a performance penalty attached to it .But it helps in making the method call polymorphycally.

so if the method is not associated with a change of state of any property of the class it is advisiable to make that method static as it improves the peformance

Regarding the use of const it is a compile time association of the value rather than at runtime. so all variables of the const get replaced by the value at compile time itself which obviously improves the performance.

Ashley John
  • 2,379
  • 2
  • 21
  • 36
  • 1
    Actually the compiler emits `callvirt` only for virtual methods; `call` is used for static methods and non-virtual instance methods. You're still right about the null check, though. – Sven Aug 11 '11 at 17:31
  • 1
    @sven:Infact the compiler emits callvirt for both virtual and nonvirtual methods.. check this link ..http://blogs.msdn.com/b/ericgu/archive/2008/07/02/why-does-c-always-use-callvirt.aspx – Ashley John Aug 12 '11 at 05:52
10

It's a matter of readability. When you make a method static you state it clear that it doesn't access non-static member variables. If you mark a variable const you state it clear that it can't (and therefore won't) be changed in code.

sharptooth
  • 167,383
  • 100
  • 513
  • 979
  • Any idea if C# has an equivalent of the `thiscall` calling convention? If so, declaring something static also avoids something being pushed on to the stack, thus making the call slightly faster. However that's all assuming something about the calling convention that I don't know for a fact. – Chris Eberle Aug 11 '11 at 05:17
  • +1; Also adding the static modifier and const parameters to a method increase the resistance to code changes that *accidentally* change state. – Merlyn Morgan-Graham Aug 11 '11 at 05:19
  • @Chris: you are essentially correct. MSIL is based on a stack machine (there are no registers; using those is left up to the JIT), so all parameters are pushed on the stack, including the `this` pointer. Using `static` methods avoids the extra push, and for example FxCop will tell you to mark any method that doesn't use instance members as `static` for performance reasons. – Sven Aug 11 '11 at 05:22
  • 1
    Cool, so I'm not full of crap. I love it when my intuition pays off. Now off to win the lottery. – Chris Eberle Aug 11 '11 at 05:22
  • @Chris: I secretly hope that when code is compiled into MSIL the compiler is smart enough to eliminate passing `this` when the method doesn't need it. I might be wrong. – sharptooth Aug 11 '11 at 05:27
  • @sharptooth: The C# compiler certainly doesn't do that. The JIT compiler might, but I sincerely doubt it. – Sven Aug 11 '11 at 06:22
  • I secretly hope that my legs are filled with jelly beans. – Chris Eberle Aug 11 '11 at 13:13
5

The other answers are correct, it's just a good practice.

But I want to show how it can benefit you. Often times, when some big method can be made static, it is a hint that there's another responsibility there, which may be best handled by extracting another object for just that task.

It also can have a chain-reaction type effect - say A calls B, both non-static. Now Resharper tells us B can be made static; we let it do its thing. Now maybe A can be made static too. Maybe A is also another responsibility entirely.

This effect has come in handy for me when refactoring older code. It let me see responsibilities and points where I could cut code out without having to sweat over every inch of text.

Paul Phillips
  • 6,093
  • 24
  • 34
  • I find this a much more pressing reason than the chosen answer, because it refers to semantics over implementation details. – 2v0mjdrl Jun 10 '15 at 07:08
2

Static class doesn't require instance to call that class and Re sharper is intelligent enough to figure out that the method can be static so people can use that method without instance.

If variable is used only for holding some real time value then it is better to convert them as constant so it save us from accidental update of that variable. It is good practice to follow and Re sharper is suggesting us the same.

Anyway if you dont like these suggestion then you can switch it off too.

Denish
  • 983
  • 1
  • 13
  • 20
1

It does so because it detects that you have no class member variables in use in a method body.

Daniel Mošmondor
  • 19,718
  • 12
  • 58
  • 99
0

Actually, I would go as far to say that JetBrains should remove their default for making suggestions to have const over static readonly. Read here: https://www.exceptionnotfound.net/const-vs-static-vs-readonly-in-c-sharp-applications/

Summary is that Const variables are VERY messy when you are dealing with multiple assemblies. If assembly A has the const X, and assembly B uses that X. Then Assembly B MUST be recompiled EACH time assembly A changes that X value. It just gives you a possible headache you do not want!

When it comes to speed? well...the compiler has come a loooooooong way and is very smart. The speed performance you gain from const is negligible in the long run.

Deleo
  • 41
  • 2