this. or underscore prefix in C#?

I've been programming in C# for 3 years and I've seen it done both ways. Normally I would say it's just a matter of opinion. But MonoBehaviour throws mud in the water by exposing lots of public fields in camelCase. So I'm wondering if one convention is better than the other for Unity.

Consider the following 2 code samples:

Code (CSharp):

1.  
2. _convention = "Underscore prefix ";         // Assign to a private field
3. string myText = _codingConvention + name;   // name looks like a local variable
4. _transform = transform;                     // No name collision when caching the transform
5.  

Code (CSharp):

1.  
2. this.convention = "This dot prefix";                 // Assign to a private field
3. string myText = this.codingConvention + this.name;   // Longer but more clear
4. this.cachedTransform = this.transform;               // To avoid a name collision
5.  

Which do you prefer in your own code?

 

Unity 5 makes caching transforms somewhat meaningless.

 

You can mark fields with the [SerializeField] attribute to expose them to the editor but not elsewhere if you're unhappy with all these public fields.

http://docs.unity3d.com/ScriptReference/SerializeField.html

 

Thanks @Glader. [SerializeField] is great for controlling what shows up in the inspector (and what gets serialized). We could probably do a whole thread on the virtues of hiding public fields, and why Unity decided not to follow that advice.

But we still have to deal with public fields like transform, gameObject, name, renderer, etc whenever we extend MonoBehaviour. Check my original post and you'll see I'm actually talking about the variable naming convention. Do you use this.myVariable or _myVariable as a general rule?

 

Would you care to elaborate?
I am in the process of porting some code to Unity 5 and I'd like to know what you're referring to. I cache various components that are used often. Partly because it was advised in Unity 4 and partly because I don't like writing 'x.GetComponent<Transform>().blah' every damn time.

OnTopic:
I prefer the this-prefix. What's far more important than which prefix you use, is making sure that you always use the same => Consistency.

 

I'm not so sure this is just a matter of preference as a variable with a preceding underscore does not necessarily signify that it is a private field, whereas an object referenced with "this" keyword will always be.... if that makes any sense.

 

I used to use underscores and this.whatever in Flash AS 2 & 3. I don't in Unity. I Just CamelCase with a lowerCase letter starting in Unity. i like my vars public so i can watch them all. This is my form of debugging to some degree..for watching things that do not throw errors but i need to/want to/like to track without the console spamming per frame.

 

i prefer and use the first one, underscores and 'this' only when needed
resharper recommends _convention for private non-const

 

As the OP I might as well throw in my opinion.

I have used both ways in Unity, and right now I prefer using 'this'. Underscore is a nice way to do things when your entire code base makes all public members PascalCase. But with unity throwing in camelCase, I think the 'this' prefix is more clear. The only caveat I've seen so far is you can't have any private members that mirror public MonoBehaviour properties with the same name, like in the _transform vs. cachedTransform example.

BTW: @hippocoder, I'm looking forward to Unity 5!

 

This claim in incorrect. Usage of this.x indicates that x is an instance member. It could be public, private, field or method.

 

Well...it's still faster to cache transform in Unity 5. It's a little faster to use transform without caching in Unity 5 compared to Unity 4, but it's not a huge difference. However, in the large majority of cases, it won't actually make any difference either way anyway, unless you're accessing transform really a lot (once a frame is not a lot).

--Eric

 

Neither of your options make sense to me, for Unity scripting.
http://forum.unity3d.com/threads/us...-not-to-be-advice-needed.282770/#post-1866473

That is incorrect; you just need to hide MonoBehaviour's members by using the new keyword.

Code (CSharp):

1. new Transform transform; Transform Transform {
2.     get {return transform ?? (transform = base.transform);}
3. }

 

Neither, underscores are ugly and typing "this." is pretty pointless. I think camelCase makes the most sense since it looks better, and most variables SHOULD be private on any particular class IMO. Therefore would it not make more sense to distinguish public variable names, since they are against the norm...

 

@Jessy, that's a really neat trick! I'd never thought about hiding base.transform.

Do you subclass MonoBehaviour with your own property overrides?

 

Correct me if I'm wrong, but Unity programmers internally uses the "m_" prefix, e.g. m_Transform for private members.

Personally I find some variation of _ clearer than this.

 

No. If C# provides a manageable way to do that, I don't know of it.

 

Fun fact: use "m_varName" as a public variable name (for some reason), and the inspector will read "VarName".

--Eric

 

Why would you make it public when you can [SerializeField]?

 

That's really not relevant; the effect is the same. The point is what you see in the inspector.

--Eric

 

@Jessy, I just mean you could wrap up your common properties in a subclass of MonoBehaviour, and then derive all your behaviours from it. So you would have a nice way to encapsulate those properties you suggested. Like this...

Code (CSharp):

1.  
2.     public class ExtendedMonoBehaviour : MonoBehaviour
3.     {
4.         new Transform transform;
5.         public Transform Transform
6.         {
7.             get { return transform ?? (transform = base.transform); }
8.         }
9.  
10.         //...
11.     }
12.  
13.     public class MyScript : ExtendedMonoBehaviour
14.     {
15.         void Update()
16.         {
17.             Transform.position += Vector3.right * Time.deltaTime;
18.         }
19.     }
20.  

 

Oh, I know what you meant. I'm saying that keeping track of a bunch of different abstract MonoBehaviour-derived classes, based on what you want cached, is impractical, at least with my current knowledge. I think the best thing to do is for Unity to just ensure that GetComponent(s) works fast.

 

GetComponent can't possibly work as fast as locally cached stuff, because a) it has to find the component dynamically and b) it has to cross the managed->native divide. (Actually... there are ways it could work faster, I guess, if the general design was changed so it didn't have to, but I think those would have significant other costs of their own, such as having to store and maintain complete copies of the Component lists for every GameObject on the managed side.)

Except where memory is a (big) concern, why not just have a single MonoBehaviour-derived class with all of the caching you might want done in it? We're talking about a few references, here. It's highly unlikely to have a significant cost.

 

It will work exactly* as fast as locally cached stuff if it is locally cached stuff. GetComponent<T> does not have to find anything dynamically; T can be a hash that retrieves a component that is dynamically updated.

All of the caching could include every type of Component you or anyone else may ever write. GetComponent<T> is the right solution.


*aside from a negligible method call.

 

Like I said...

Fair enough... The way I write my code that's not the case. I do my best to keep the individual components as unaware of each other as reasonably possible, so they don't communicate directly very often except where one "owns" another.

 

At one point I had considered caching every component on Awake and overriding GetComponent<T> so I could lookup components by their interface. Then I would need a RebuildComponentCache() method in case a component changed. But that seemed like overkill.

 

After a talk last week, in which we assessed that all other languages we use got case right, and Microsoft did not, my team started using camelCase for properties, and fields that do not back properties, along with _camelCase for backing fields. Methods are still PascalCase, which allows us to give the same name to properties and methods, even though the compiler is too stupid to permit it.

This has the benefit of making static methods look correct, when you have a property/field named the same as a type, in a class, along with use of a static method from that type. Also, our code looks just like Unity's now.

 

I've also been using camelCase for regular fields and properties, and _camelCase for backing fields.

I only use the this keyword when I'm accessing variables that have the same name on different objects (pretty much the intended usage). Using it constantly seems like a lot of visual clutter.

Really, I see more people using _camelCase to designate private fields than I see liberal usage of "this". I don't see how overuse of the keyword benefits the readability of the code, or comes anywhere close to the power of a proper naming convention.

For example, I can look through a list of method names, fields, and properties, and determine which is which without knowing anything but the names. There is no way "this" can do anything similar.

 

this

 

How do you deal with parameters that have the same name as fields/properties, then?

 

I hate the underscore. It's annoying to type and it looks dumb and it gives me flashbacks to C++ where it was heavily recommended that you NEVER start anything with an underscore or especially a double underscore because those were reserved for special system defines and Microsoft black magic.

I just use camelCase for member variables, PascalCase for methods, and usually PascalCase for properties though I've been considering just using camelCase for them. In constructors and similar things I give the parameters the same name as the member variables and use "this" to assign them, even though Resharper yells at me about it.

 

What about backing fields?

Agreed, but C# has backing fields and we need to deal with them for now. Backing fields are stupid. Haxe and Swift don't use them, and I suspect C# won't in the future either. But that future is not now, and judging from the past, that now will be 10 years later in Unity than it is .NET.

 

I do the property in PascalCase and the backing field in camelCase. Though honestly I don't tend to use properties that require backing fields that often... I usually use auto-properties with a public get and private set, and only allow setting through a method, and if there's something really complicated that has to happen in a get I usually just use a method instead. I always try to avoid properties that alter the state of the object in weird ways especially if it's during a get because then all kinds of strange things can happen when you try to view the value in a debugger or in Unity.

 

What about them? They're just variables like any other.

Also, if you're using PascalCase for methods then it makes perfect sense to also use them for Properties, since properties are just dressed up methods anyway.

 

Okay, how about this?

Code (CSharp):

1. namespace UnityEngine {
2.     public static class ComponentExtensions {
3.         public static T GetComponent<T>(this Component component, ref T cache) where T: Component {
4.             // null-coalescing operator doesn't work with serialized UnityEngine.Object
5.             return cache ? cache : (cache = component.GetComponent<T>());
6.         }
7.     }
8. }

Code (CSharp):

1. namespace UnityEngine {
2.     public abstract class MonoBehaviourWithCached<C>: MonoBehaviour, ISceneInitializable where C: Component {
3.         #region ISceneInitializable implementation
4.         public virtual void Reset() {cachedComponent = GetComponent<C>();}
5.         #endregion
6.  
7.         [SerializeField] C _cachedComponent; protected C cachedComponent {
8.             get {return this.GetComponent(ref _cachedComponent);}
9.             set {_cachedComponent = value;}
10.         }
11.     }
12.  
13.     public abstract class MonoBehaviourWithCached<C1, C2>: MonoBehaviourWithCached<C1>
14.     where C1: Component
15.     where C2: Component
16.     {
17.         #region ISceneInitializable implementation
18.         public override void Reset() {
19.             base.Reset();
20.             cachedComponent2 = GetComponent<C2>();
21.         }
22.         #endregion
23.                
24.         [SerializeField] C2 _cachedComponent2; protected C2 cachedComponent2 {
25.             get {return this.GetComponent(ref _cachedComponent2);}
26.             set {_cachedComponent2 = value;}
27.         }
28.     }
29.  
30.     public abstract class MonoBehaviourWithCached<C1, C2, C3>: MonoBehaviourWithCached<C1, C2>
31.     where C1: Component
32.     where C2: Component
33.     where C3: Component
34.     {
35.         #region ISceneInitializable implementation
36.         public override void Reset() {
37.             base.Reset();
38.             cachedComponent3 = GetComponent<C3>();
39.         }
40.         #endregion
41.        
42.         [SerializeField] C3 _cachedComponent3; protected C3 cachedComponent3 {
43.             get {return this.GetComponent(ref _cachedComponent3);}
44.             set {_cachedComponent3 = value;}
45.         }
46.     }
47. }

Code (CSharp):

1. class NewBehaviourScript: MonoBehaviourWithCached<Renderer, Rigidbody, Transform> {
2.     new Renderer renderer {get {return cachedComponent;}}
3.     new Rigidbody rigidbody {get {return cachedComponent2;}}
4.     new Transform transform {get {return cachedComponent3;}}
5. }

 

I use camelCase for properties in general to help seperate them from methods by functionality. If it's simple, I'll do it in a property when it makes sense to, but if the property gets over 10 lines for a getter or setter, I start to consider using a method instead, or at least implementing some of the functionality in a method. Which usually winds up being something I want to reuse anyway.

Properties are just dressed up methods, but they do save on some boilerplate code. I try to avoid over using them, but I like to be able to differentiate in Intellisense by case, even though there's a visual cue too. I scan lists very quickly. For others, it might not be as important to have the textual cue.

 

Interesting read. Here is what I use.

Methods, classes and types use PascalCase.
Variables and properties use camelCase.
Backing variables use an _underscore.
Local cached components use my is the name collides with Unity.

The underscore for backing variables is to let me know I shouldn't be accessing it outside of its defined properties. I typically ensure there is at least one private setter so I am never accessing a backing variable directly.

Using the this keyword is pretty much restricted to the cases where I have to use it. Mainly in methods and constructers where there is name collision between the parameters and the member variables. Otherwise I don't use this.

 

Ok. Everybody, stop what you are doing! @Jessy has just created a generic MonoBehaviour that caches whatever components you want! I think we should take a moment to acknowledge how awesome that is

Seriously @Jessy, it looks good. It might be overkill for my purposes, but I'm anxious to see if anyone will put it to good use in a real project. Nice job.

 

Is there a way to make MonoDevelop or Unity warn you about accessing public or private iVar without "this"?

I'm using underscores right now and I'd like to go away from it. But I often forget to put "this" before iVars.

 

Why do you use the word "forget"?

https://github.com/raywenderlich/swift-style-guide#use-of-self

 

I code natively for iOS and Xcode highlights iVars. MD does not. Do you know if Xamarin does?

And I'm also going to use "self" in Swift (when I'll move to it from Obj-C) because I need to see clearly when I'm modifying properties/iVars in my methods.

http://stackoverflow.com/a/24215657

 

You're talking about manually implementing an IDE feature, every time you type a variable, by replacing it with source code. The concept is ludicrous; it's akin to not being able to name variables, for example. If the IDE doesn't give you good enough feedback regarding variable ownership, file a radar.

 

Not everyone uses or likes to rely on a full-featured IDE, and even when you are using one mileage varies depending on features, language, environment, etc. Furthermore, an IDE's features can only tell you what is, they can't tell you intent, and that can matter with large teams and/or long running projects.

Having said all of that, I broadly agree - I'll personally make my tools do as much of the work as they can so that I can put my effort elsewhere and for me that means leveraging my IDE and my language and not bothering to add annotations myself. In other cases those annotations themselves might be a tool that saves more time than it costs. Horses for courses.

 

Style aside, prefixes of some sort are really useful for intellasense or grouping things together or identifying a specific use case.

I use m_ for private variables set at or mutable at runtime only, so basically m_ means runtime state.

 

I'm implementing a language restriction that doesn't exist. Ruby has @-prefixed vars - it makes things super clear.

I would like to have an IDE-forced auto-this though.