Mixedrealitytoolkit-unity: Inconsistent use of explicit public/private modifiers for methods

Created on 15 Sep 2016  路  9Comments  路  Source: microsoft/MixedRealityToolkit-Unity

I guess this is more of a Nit for formatting, but it is also good for helping other programmers know what methods they're allowed to safely call from other classes.

bad:

void Start()
{
    //code
}

good:

private void Start()
{
    //code
}

Most helpful comment

We should encourage people to use explicit public/private during code reviews. I think it's helpful for people coming from different programming languages to know exactly what the access level is. I don't think we need to do a mass code update, just update the code when making other changes like @HodgsonSDAS.

All 9 comments

I think there's a good discussion to be had here. By default, Unity's generated C# scripts follow your "bad" pattern, which is why (almost?) all of our scripts leave modifiers off Start, Awake, Update, etc. I can't find any Unity code style docs which mention this design decision either way.

You can actually change the generated code templates to match your preferred pattern.
Simply update C:\Program Files\Unity HoloLens 5.4.0f3-HTP\Editor\Data\Resources\ScriptTemplates\81-C# Script-NewBehaviourScript.cs.txt

This is also helpful when you forget to put your companies licensing information at the top.

That's true; I've made some edits there already for formatting.

While true, should we force/can we expect every contributor to make this change? Is there a reason Unity leaves these modifiers out by default, and do we want to follow their pattern or our own? I'm not leaning too much in either direction at the moment, mostly just trying to start the conversation, since this will probably affect almost every script in the repo.

There's no real reason why unity leaves them out, and it doesn't hurt others for forcing them to use it (they can make that decision in their own code). It's really only enforced to help others understand that a particular method shouldn't be used outside of a class.

I've only been updating them when I touch a class that needs to be changed.

We should encourage people to use explicit public/private during code reviews. I think it's helpful for people coming from different programming languages to know exactly what the access level is. I don't think we need to do a mass code update, just update the code when making other changes like @HodgsonSDAS.

wow...I didn't know you could edit the default script. Very useful.

I think we're mostly agreed to try and encourage explicit calling permissions.

Do we have a formatting document for people to follow?

Not that I know of.

Was this page helpful?
0 / 5 - 0 ratings