Newbie C# question about static

R

  1. often suggests that "Method can be made static". I'm still new to C# (I'm

coming from VB).

I understand what a static/shared method is and where you would use
explicitly use one. However, is there any advantage in making all methods,
including private methods, that do not refer to instance variables static?

I would have thought that this is unnecessarily exposing the classes'
interface, however, I have no formal CS background and would welcome some
enlightenment on this.

Many thanks in advance
Jeremy



5 comments
Comment actions Permalink

There are a few good reasons for R# to flag you on methods that could be static, the most important of which is:

(In general) A static method has a code smell that suggests a missed opportunity for increased object-orientation. A static method that takes one or more types as arguments usually can and should be moved to be an instance method of the most sensible argument's type. There are a whole of slew of benefits that can come from moving the method, one of the most immediately beneficial of which is that doing so puts the logic in the most obvious place where you (in the future) and/or other developers (now and in the future) would look to find it, which aids reuse and avoids rework. After all, (at its most fundamental) object-orientation is about co-locating functions with the type with which each is most closely related.

PS - you can even find cases of this that are actually buried right in the middle of other functions (which may or may not be static.) If, within a larger function, you notice a cohesive section of code that keeps accessing properties on and/or calling methods of a given type, you may well be able to extract that section into its own method. If the resulting extracted method "makes sense" on its own, you can then move it to the appropriate type.

0
Comment actions Permalink

Hi Jeremy,

That's pretty much what I understood - that one should try to expose as
little of the class interface as possible. It just seems counter-intuitive
that R# keeps suggesting that I should change instance methods into static
methods.

I tend to use static methods in utility classes only, and avoid creating
static variables within a class.

Regards
Jeremy



"Jeremy Gray" <no_replay@jetbrains.com> wrote in message
news:17708387.1204152105891.JavaMail.itn@is.intellij.net...

There are a few good reasons for R# to flag you on methods that could be
static, the most important of which is:

>

(In general) Static methods are a code smell that suggests a missed
opportunity for increased object-orientation. A static method that takes
one or more types as arguments usually can and should be moved to be an
instance method of the most sensible argument's type. There are a whole of
slew of benefits that can come from moving the method, one of the most
immediately beneficial of which is that doing so puts the logic in the
most obvious place where you (in the future) and/or other developers (now
and in the future) would look to find it, which aids reuse and avoids
rework. After all, (at its most fundamental) object-orientation is about
co-locating functions with the type with which each is most closely
related.


0
Comment actions Permalink

Hmm. Let me see if I can find a better way to describe this, as this really isn't at all about tying to "expose as little of the class interface as possible".

Utility classes are just that, and are certainly not uncommon. If you have some completely pure utility classes you can fill them full of static methods and mark the class itself as static so that it is clear that it is a pure utility class. Utility classes aside for the moment, the reason that you want to be informed by R# when a method could be made static is because a static method in a non-static (and therefore non-utility) class is generally a code smell.

Note that I just say "generally", as there are certain exclusions to this, such as static methods providing access to singletons, though there are certainly people that also say that singletons are a code smell, albeit for different reasons. :)

Anyway, back to the code smell. A method that even can be made static can be made static because it doesn't operate on any of the fields of the type in which it is contained. If it doesn't operate on any of the fields of the type in which it is contained, it is probably in the wrong place and/or is structured in a manner that runs counter to one of the key tenets of object orientation (which I mentioned in my earlier reply). If it acts on a type within your object model (eg. by taking it (and possibly other types) as an argument) it should be converted into an instance method for that type. If it doesn't act on a type within your object model (eg. perhaps all of the arguments it takes are .net framework types), it should probably be moved into a static/utility class somewhere within your codebase (and, given the latest C#, you can consider making it an extension method if you like). Changing it to be an instance method of one of the types within your object model makes your code more object-oriented by placing the function next to the data on which it operates, and has a bunch of benefits in terms of reuse instead of duplication, etc.

I know that I just partially restated myself in the above, but hopefully we're heading in the right direction. Let me know.

0
Comment actions Permalink

I really appreciate the time you're taking out to help me this. I've been
(still am) an amateur coder (hacker) for many years. This is probably why
I'm having so much difficulty getting my head around this. In fact if it
wasn't for R# I would never have stopped to ask the question - so perhaps
JetBrains should market this product as a learning tool as well as for all
its amazing functionality. Thinking about it, it would be extremely useful
if one had an option to offer explanations for why R# is making the
suggestions it does.

Lets say I have a class that uses Linq to SQL to access the data. Based on
what I've read so far (for asp.net apps), I should try to keep a hold on the
DataContext for as little time as possible. Thus I have seperate methods for
Select, Insert, Update, Delete. Select creates a new DataContext and
retrieves the data and passes it to the web page for display. Insert, Update
and Delete methods receive back the altered data from the web page and
create new DataContexts whereupon they submit their changes back to the data
store.
In this setup there is no coupling between Select, Insert, Update or Delete
and no class scoped variables are used.

List Select { var db = new MyDataContext; return (from c in db.MyTable select c).ToList(); } void Update(MyTable entity){ var db = new MyDataContext; db.Attach(entity,true); db.SubmitChanges; } If I were writing the same code for a Windows Forms app I would use a class scoped variable for the DataContext, and have a single Save method which would use this variable to submit the changes. class DataAccess{ DataContext db = new DataContext(); List Select { return (from c in db.MyTable select c).ToList(); } void Save(){ db.SubmitChanges } I'm having trouble understanding what the logical difference between the two examples are. In my mind, both classes wrap the functionality of obtaining and updating data. Therefore the methods in both examples should be instance methods - which (if I have understood you correctly) concurs with your statement "If it acts on a type within your object model (eg. by taking it (and possibly other types) as an argument) it should be converted into an instance method for that type". The fact in the asp.net example that the type is intentionally kept within the routine and intentionally not allowed to interact with the rest of the class shouldn't make it any less an instance method. But it is these methods that R# is flagging that could be converted to static. How should I write the asp.net examples above to comply with your statement "it is probably in the wrong place and/or is structured in a manner that runs counter ...". Again, many thanks for the time you're taking to help me with this. Regards Jeremy "Jeremy Gray" ]]> wrote in message
news:6570738.1204306579222.JavaMail.itn@is.intellij.net...

Hmm. Let me see if I can find a better way to describe this, as this
really isn't at all about tying to "expose as little of the class
interface as possible".

>

Utility classes are just that, and are certainly not uncommon. If you have
some completely pure utility classes you can fill them full of static
methods and mark the class itself as static so that it is clear that it is
a pure utility class. Utility classes aside for the moment, the reason
that you want to be informed by R# when a method could be made static is
because a static method in a non-static (and therefore non-utility) class
is generally a code smell.

>

Note that I just say "generally", as there are certain exclusions to this,
such as static methods providing access to singletons, though there are
certainly people that also say that singletons are a code smell, albeit
for different reasons. :)

>

Anyway, back to the code smell. If a method in a non-static class can be
made static, it is probably in the wrong place and/or is structured in a
manner that runs counter to one of the key tenets of object orientation
(which I mentioned in my earlier reply). If it acts on a type within your
object model (eg. by taking it (and possibly other types) as an argument)
it should be converted into an instance method for that type. If it
doesn't act on a type within your object model (eg. perhaps all of the
arguments it takes are .net framework types), it should probably be moved
into a static/utility class somewhere within your codebase (and, given the
latest C#, you can consider making it an extension method if you like).

>

I know that I just partially restated myself in the above, but hopefully
we're heading in the right direction. Let me know.


0
Comment actions Permalink

No worries, I'm more than glad to help out.

The very first question I would need to ask is regarding your first block of example code with the Select and Update methods. What class (or classes) are these methods in?

A second question I could get out of the way right now would be regarding the second block of example code where you retain a DataContext across method invocations, even though in your earlier text you mentioned wanting to only hold on for a DataContext object as briefly as possible. Is there a reason you are holding onto it across calls, or is that just the way the code shook out when you were experimenting with instance vs. static approaches?

A third question that I have for you is regarding how the first block of example code uses a MyDataContext class and the second block of example code just uses a DataContext. What is MyDataContext, and why is it not being used in both cases?

One other comment I should put forth right away is this: When you mention "In my mind, both classes wrap the functionality of obtaining and updating data. Therefore the methods in both examples should be instance methods" it is worth nothing that there is no direct "therefore" association involved, at least not with regard to whether or not the methods should or must be instance methods, and that may be where the disconnect is. You can wrap functionality to provide a layer of abstraction a whole bunch of different ways, but we should be able to get the instance vs. static thing sorted out for you without having to get too deep into the rest of your app. I may have to present some examples that work for the purposes of covering the instance vs. static issue but which might have some downsides when used in a real application, and I'll try to be clear when I do that.

While it would be very helpful if you could respond to the three questions posed in this reply, one quick thing I can point out that might get things moving in the right direction is this: Your Update method in your first example is essentially a static function. Even though it isn't marked as static, it is static in that it does not operate on any of the instance members of the class in which the method is placed. This means that it could be put anywhere in the codebase, and that itself is part of the problem. As a pure function, it is by definition not object oriented, and if object orientation is what you are pursuing as a technique to make the development of your application "better" (you could certainly choose other approaches but in general it looks like you are trying to go OO), this Update method should changed and moved so as to be co-located with one of the types on which it acts. Depending on the style of design that you would like to pursue, and based solely on what the function itself currently does (which is to operate on two of your types, MyTable and MyDataContext), this points towards two natural destinations for the Update method:

1. In the MyTable class. This would result in

public void Update() {
var db = new MyDataContext;
db.Attach(this, true);
db.SubmitChanges();
}

Which has its own upsides and downsides.

2. In the MyDataContext class. This would result in

public void Update(MyTable entity) {
Attach(entity, true);
SubmitChanges();
}

Which has arguably less downsides given that in the original Update method you provided the MyDataContext class was the one interacted with most often within the method (and because lots of things can be Updated, you probable have more MyTable-derived classes than you have MyDataContext-derived classes, and you'd prefer to write Update only once if at all possible. (In fact, if OO is applied in full, each conceptually unique operation that can be performed by your application will be implemented once and only once within your codebase.)

Either of those approaches would take that pure Update function and apply a degree of object orientation to it. There are pros and cons to each approach, of course, but does this at least help shed a bit more light on things so far?

Jeremy

0

Please sign in to leave a comment.