Prefixing Instance Fields?
Posted by Davy Brion on October 26th, 2009
I was talking to a friend the other day about prefixing instance fields. His opinion was that by prefixing instance field names (with an underscore for example) you have the benefit of immediately showing the difference in scope between the usages of a local variable in a method, or the usage of an instance field.
There is indeed a legitimate possible problem here: any possible mutation of an instance field in a method can have an intended or unintended impact somewhere else in the class since the mutation will outlive the duration of the method call. So it is important to be aware of this and that you are entirely sure that any possible mutation is actually intended and that no unintended mutations will occur because of some faulty code in a method. I don’t however think that prefixing field names is the best solution to avoid this problem. There are two practices that i recommend to easily avoid unintended mutations of fields in methods.
The first is to use as few fields as possible. To avoid any misunderstandings: i’m not saying that you shouldn’t use fields! I’m just saying that fields need to justify their existence. Fields inherently make up the state of an object, so if you don’t need a particular piece of data to hang around longer than say, the duration of a method call, then don’t put that data in a field. If you need to access that data in another method that will be called from the originally called method you can simply pass it to the other method with a method parameter. If you notice that you’re frequently passing the same pieces of data to a couple of methods, then you probably need to introduce a parameter object. Simply put: do not use fields unless the data you want to store in them are an inherent part of the object and the data needs to hang around for a while.
The second practice is too keep your methods short and focused. If you’re working with large methods, then i can certainly understand why you would want all the help you can get to easily spot mutations of fields. There’s only one question: why are you working with large methods? There are truly very few legitimate reasons for not splitting up large methods into separate reusable methods. When you use many small methods instead of large methods, it’s much easier to keep your focus on what any particular piece of code is supposed to do, and what it’s really doing. And if your methods are short, you will very easily see when a field is being used instead of a local variable.
The combination of these two approaches leads to a lot less unintended mutations of fields. And as a bonus, you get a lot more readable and maintainable code as well. If however you still like to get some visual help in identifying field usage in methods, you can enable the Color Identifiers feature in Resharper (available in the Options window under Code Inspection – Settings – Color identifiers) which will result in fields being represented with a different color from regular variables.
What do you think about this? Do you still prefer to prefix field names, and if so, why?
October 26th, 2009 at 8:50 am
I don’t think it really matters that much. The visual clue is the reason I use them, but I would have no problems not using them if a project demanded that. I think what is more important is that you stick with one style throughout the project making the code base more consistent.
October 26th, 2009 at 11:41 am
Old C++ head here! So I can’t stop using underscores, just habit…
I think method structure & size are more important, just apply SRP in moderation along with obvious naming and you can’t go wrong
also try not to use ‘if’ statements
October 26th, 2009 at 2:28 pm
It’s a very good point regarding field-itis. I recently pair programmed with a junior developer who was using a lot of unnecessary fields and it was something that stuck out like a sore thumb.
The reason I noticed was not down to some inbuilt software engineering smell detector; it was purely down to the fact that the unnecessary use of fields frequently made the code harder to follow. You have to jump around looking to see what the field is, who accesses it and so forth.
It should also be noted that NDepend’s LCOM metric will flag up classes that have fields accessed by a small proportion of its methods. This could be useful for identifying classes with this problem.
October 26th, 2009 at 2:52 pm
+1 for NDepend’s LCOM metric (Lack Of Cohesion of Methods: http://www.ndepend.com/Metrics.aspx#LCOM)
Still, a visual indication is nice to have. So if you do not use Resharper, prefixing them with an underscore doesn’t seem at all that bad to me. Then again, why would one not use Resharper?
October 26th, 2009 at 3:23 pm
I put underscores mostly because when I want to access a member variable, I just type _ and get a list of nothing but member variables. Many moons ago another reason was to easily make them stand out against local variables, but R# fixes that as you pointed out. Last one I can think of (but not really a special reason for me) is to avoid naming clashes with local variables – typing ‘_’ is so much easier than typing ‘this.’
In the end, I don’t think it’s something worth thinking about all that much
October 26th, 2009 at 3:39 pm
It has taken me many years to get to the style I use. I have been programming in OO since around 1990. I agree with the two recommendations in your blog, but I see them as a best practices issue that is orthgonal to naming. I use the prefix “my” for instance-based field names and the “our” prefix for statics.
October 26th, 2009 at 3:57 pm
I don’t use underscores. With short methods, the intent is always clear, and resharper usualy help to spot the rare cases where you mix up. In this case, I try to find better names so that there is no more ambiguity.
October 26th, 2009 at 4:51 pm
I detest warts! Prefixing with underscores(warts) adds little to no value to helping a developer understand the code. If I really want to know the scope of the variable, Resharper(Ctrl+B), VS(goto definition) and Reflector(click on the variable) all make the experience easy enough. Remembering to type “_” is just as bad as typing “this.”.
+1 for the 2 general practices about using fields.
The only place I use warts is writing BDD specifications. Mostly because the spec runners tools “can” generate a friendly version of the specifications.
October 26th, 2009 at 5:47 pm
Using fields for passing data between methods can be a code smell, if over used. Done properly, the code can become so much more readable. If you find only a few methods using the same instance variable perhaps an extract class refactoring is an order?
The naming thing doesn’t really matter, as long as you’re consistent.
October 26th, 2009 at 10:00 pm
@Dennis – While I agree with your thoughts about “this”, and also give a +1 for the rest of Davy’s post, here’s why you are wrong about the first paragraph of your comment:
Resharper(Ctrl+B) ==> requires an extra click to see information that could be there at all times. Not everyone installs R#. Navigating back is an extra step. Depending on where you navigated to, navigating back can prove problematic given VS behaviour.
VS(goto definition) ==> requires an extra click to see information that could be there at all times. Slow. Navigating back is an extra step. Depending on where you navigated to, navigating back can prove problematic given VS behaviour.
Reflector(click on the variable) ==> requires an extra click to see information that could be there at all times. Reflecter is way too late. This insight is needed when working with source, not when reverse engineering compiled assemblies.
+1 for _. All the benefit of “this.”, but with less typing. None of the overhead of going hunting for information interactively when it could just _be there_.
October 26th, 2009 at 11:48 pm
I use underscore, ._. and never this. While I hate to put unnecessary fields to class (they make “class-wide-global-variables”, and make class logic hard to understand), I do use underscore to keep me focused on code logic by know which is what.
October 27th, 2009 at 4:10 am
Something related that I have pused with MS only to receive “we could but, …” is property scopped fields i.e.
public int Count
{
int count = 0;
get { return count; }
set { if (CountIsValid(value)) {count = value;} else {throw new InvalidArgumentException();} }
}
This would remove the need to most of the fields in most classes
October 27th, 2009 at 9:52 am
It depends on what the accessbility of the field is. If we are talking about private fields it does not matter what you use, as long as your entire team uses the same convention. If we are talking about a protected field, according to the Framework Design Guidelines, they should be PascalCased (upper camel case). Public fields are bad IMHO, it’s like a cop querying your brain for your name and address instead of asking you to show him your ID.
@Wayne: a better naming for your CountIsValid() method would be to start with Is as in IsValidCount(), see also the Framework Design Guidelines or on MSDN Library.
Also like Davy mentions there are several reasons why you should not use complex properties and it is because it is possible that consumers of your API will use the property in a for-loop for example without caching the returned value of a property in a local variable first:
for (var i = 0; i < someClass.Count; i++)
{
Foo(someClass.Count);
}
Because you dont expect too much complexity in the getter for Count property. But when a developer sees that it is a method they tend to cache it in a variable first:
for (var i = 0; i < someClass.GetCount(); i++)
{
Foo(someClass.GetCount());
}
They will probabyl write this:
var count = someClass.GetCount();
for (var i = 0; i < count; i++)
{
Foo(count);
}
I think it has to do with psychology
October 27th, 2009 at 6:14 pm
[...] Prefixing Instance Fields? – Davy Brion [...]
November 10th, 2009 at 3:46 pm
I think this is a lot more to do with tooling than people realize. In the days that hungarian notation was favoured, figuring out the type of a variable was often painful, especially when using the windows API. Equally, these days, there are two things that encourage people to use underscores:
1) Instance fields don’t get coloured differently from local variables in the average IDE.
2) Putting an underscore in front groups them together in class browsers.
It wouldn’t be hard to improve the tooling in these areas, of course…
November 17th, 2009 at 3:51 pm
Conventions are the biggest reason for using an underscore, I have no problems with it. The other reason that I use underscore is for CLS compliance. My project (unfortunately) has libraries written in both C# and VB.NET, and field names that only differ in case to property names are not CLS compliant.
November 18th, 2009 at 12:54 am
It’s always bugged me about VB (not that I’ve coded much in VB) with regard to its me. convention. The field (or member) of a class is part of the class instance so I’ve always found using me. or this. redundant. I prefix private fields with underscores so I can spot them in the ide and on printed paper.
I also declare them at the bottom of the class declaration, but that’s another story.