Opinions about C# class structure construction and best practices

Hi,

I am a senior programmer coming from best practices using ASP.NET MVC. I am using Unity for like 1 year but i struggle a bit about applying best practices with it. This is probably due to its nature and the way it work (such as being a "component pattern").

I would like to know your opinion about how you guys structure your script classes. An example we could use to describe better the problem i face are the properties in a class. Unity serialize by default public fields and properties.. but if you want private field you have to specify it manually. So the only way i found to use property in a "clean way" is to do this:

[SerializeField]
private string someValue;

public string SomeValue
{
get { return someValue; }
set { SomeValue = value; }
}

My concerns are i feel it is a lots more job to do this when you have tons of properties. Are you guys would prefer doing this (over) or for this case simply go the simple route of writing:

public string someValue;

Thanks!

 

Well, you seem to be struggling with something that I ran into when I started transitioning over to Unity. Unity's C# Monobehaviour scripts don't utilize mutators the same way that most standard C# programs do. Unity is used to treating simple public variables as properties, and ignoring declared mutators in its component structure.

In my earlier C# education, I loved using C# properties by declaring mutators and using them to gate access to private variables. It was very rare for me to ever declare public variables. Why bother when the same functionality can be had with nice, cleanly defined mutators that give me fine-tuned control over what is accessible and in what way? Being able to perform additional operations or testing right inside the mutators was icing on the cake. My infatuation with them was reinforced by similar functionality that I had previously learned in AS3, which has a very similar structure for declaring and using properties.

While using mutators in Monobehaviour scripts is possible, it isn't encouraged, and they won't show up as "properties" in Unity's internal UI. (the way that public variables will) This is especially problematic when dealing with Unity's undo/redo system. If you want the more fine-tuned control over public variables, I strongly recommend looking into writing custom inspectors. These can provide some of the same fine-tuning for gating access, and allow you to customize who public variable "properties" are displayed in the Unity UI.

 

I have scriptable objects that are just this repeated twenty times. It's really not ideal.

Personally I think it highlights how properties are poorly thought out/implemented. Sometimes they are a variable with specific read/write access, while other times they are just functions that masquerade as variables. I assume this is a good chunk of the reason the serializer doesn't even try to touch them (plus some limitations in reflection magic I assume).

 

Not serialising properties is an annoyance. But it's only a minor one. If you really need it you can write custom property drawers.

In many cases properties can't be serialised anyway, you need both a set and a get for serialisation. And they both need to do the same thing with the data. Throw in any side effects into your properties and suddenly the inspector is spinning into infinite loops and doing weird things with your data. Serialising the backing data is often a better idea anyway.

I often treat the serialised values in the inspector as initial values only. Combine them with Awake to make a sort of pseudo constructor.

 

I leave my coding standards at work. But thats mainly because the coding standards at work are anal retentive.

Dont bother with getters/setters and privates personally. Just code noise most of the time.

 

That what i was thinking. I figured i was taking way too much time making my code right to the standard i mostly see..but in the end i think its more important to have a coherent/easy to read/maintenable code. Putting getters and setters everywhere the way i must do it with Unity including SerializeField attributes make a lots of noise. In the end its less readable, less maintenable and doesn't give any added value.

 

My thoughts exactly.

 

If you need them throw them in. But if you are going to have a public setter and a public getter that do nothing except point to a private field, don't bother.

Inside the unity framework it only makes sense to use properties where the properties actually do something. General Unity coding convention treats fields and properties the same. So you won't need to do any refractoring if you decide to change a field to a property later.

 

This is a good general rule. Also just a note, if you're creating properties to maintain consistency, you can short hand them into:

Code (CSharp):

1. public int SpecialInt
2. {
3.      get;
4.      set;
5. }

 

At that point it's definitely not valuable, unless that's just how your naming conventions work best. Nine times out of ten, all I ever need is a public getter to a private field. Hell, if there was a way to make the inspector write to readonly variables, that would be perfect.

 

naming is a poor reason for choosing one over the other.

Only decision that should affect it is 'do I want to do other stuff when I get/set the property'

int example;
public int Example
{
get {return example;}
set {
example = value;
NotifyListeners();
}
}

 

I agree with everyone saying to use properties only when necessary. When I started using Unity, I used to use them all over the place. That's because at university professors always told us to avoid public fields and they were doing so to prevent us from using public fields everywhere and to get us familiar with properties, while they should have just explained us when and why to use one over the other.

I realized that it's just an unnecessary effort, similar to premature optimization, and now I just begin with a public field and then, if required (usually when I need to do something when setting a new value to the field), I switch to a private field and a property.

Ditto.

 

Forget what you learnt and go all public crazy!

 

I shorten it to one line since vertical space is more valuable.

Code (CSharp):

1. public int PublicVar { get; set; }

 

Or want something like
public blabla blablabla {get;private/protected/internet set;}

 

This is shorter. And serialisable.

Code (CSharp):

1. public int PublicVar;

Now that's living dangerously. Do I need to pull out my lecture about keeping your intimate bits private? There are some parts of a classes body that just shouldn't be exposed in public. They should only be handled in private.

 

Happy?

Code (csharp):

1. [SerializeField]
2. private int m_variable;
3.  
4. public int Variable {
5.     get { return m_variable; }
6.     set { m_variable = value; }
7. }

 

Yes of cause. What i do most of the time is making anything i need to reach from other scripts public (most of the time). Everything else private (and then i dont need any get/setter, again most of the time). Protected for inheritance of cause. I know thats not always the "right" way to do it but it saves me time and and makes my scripts more readable. And after 4+ years of developing in Unity it has never really been a problem.

 

I haven't quite achieved world domination yet if that's what you are asking.

But seriously, it's only worth using properties if you need to modify the value. Or if you need to maintain different levels of access for the setter and getter.

Everything else is just typing characters for characters own sake. There is no practical difference.

(Okay that's a slight lie. Properties and fields behave differently under reflection. But that's another topic.)

 

If you're working within the same assembly, here's a quicky...

Code (CSharp):

1. internal variable;

 

I tend to not use properties at all. I've reverted back to the old school Getter/Setter approach.

Code (csharp):

1. GetSomeValue(); SetSomeValue( float value );

The benefits of being able to use fields in the inspector outweighs the semantic sugar you get from properties. So if I really need to provide public access to something, I just roll it into a method.

There's also something clean and correct feeling about writing these as actual methods, it feels honest.

 

Old habits and all that. Techniques I've learned over the years for easing debugging are hard to ditch again.

 

Thanks everyone for the comments.

And yeah i wouldn't go as far as going public for everything. As someone else stated some fields need to be private since they are used by the internal class and you don't want external class to access them. I also have to point out that the shorten get and set also called as magic properties can be a problem with some assets which use reflection. This kind of extension intent that you will use fields mostly because that how Unity serialisation work best. Just my 2 cents.

 

There is a slight performance penalty for doing this though. The compiler does some fancy inlining of properties to match the performance of fields unless those properties have side effects such as through method calls. Methods won't be inlined.