Coding style - let the flame wars begin ;-)

[wilhelmberg]

Disclaimer: opinions are my own, suggestions/additions welcome!

@mapbox/games

Building on @david-rhodes's Mapbox Unity SDK Review from Nov 2016 every now and then we've been discussing how to format our code.

I think it's again time to discuss and decide on at least a few - especially with the current talks about opensourcing. We should have consistent formatting/naming when going public.

Let's start with my favourite:

tabs vs. spaces

I, personally, will never understand why anyone would ever want to use spaces - yeah, sue me 😏 But, if we decide to use spaces I have no problem with that. I already had changed all my editors and Visual Studio to spaces - yet we often end up with mixed tabs/spaces. I've never used MonoDevelop that comes with Unity, but my hunch is that's were the tabs might come from.

Please enable display of whitespaces in your IDE/editor - tabs vs spaces will pop right into your eye!

---

• I like private fields to have an underscore and start with a capital letter private int _Level;

• We should adopt as much/as little we want from MSDN: .NET Framework 4.6 and 4.5 Naming Guidelines

  • might make automated checks with StyleCop/FxCop at lot easier

• I like variable names as verbose as possible CanonicalTileId vs ctid

• I like to have code that belongs together as compact as possible - opening brackets on same line and if it's a very short statement sometimes even everything on one line:

  public int Zoom {
     get { return this.zoom; }
     set { this.zoom = value; }
  }

  vs.

  public int Zoom
  {
     get
     {
         return this.zoom;
     }
     set
     {
         this.zoom = value;
     }
  }

• I like to have different methods/properties separated by two lines to easily spot where one ends and the other starts

  public void Method1() {
     DoStuff();
     DoMoreStuff();
  }
  
  public void Method2() {
     DoStuff2();
     DoMoreStuff2();
  }

  vs.

  public void Method1() 
  {
     DoStuff();
     DoMoreStuff();
  }
  
  public void Method2() 
  {
     DoStuff2();
     DoMoreStuff2();
  }

• I prefer events over IObserver 😏

• I prefer to have braces even for one liners. It's just so easy to overlook this when refactoring and adding a second statement:

  if(condition) {
  doStuff();
  }

  vs.

  if(condition)
  doStuff();

• I prefer to switch order when evaluating conditions to not accidentally asign instead of comparing - although I must admit that is a habit from a time when IDEs didn't highlight those errors automatically

  if(null == aInstance) {...}

  which avoids if(aInstance = null) <- notice only one = vs.

  if(aInstance == null){...}

---

Below are all my code formatting settings from Visual Studio. I wanted to check if there are equivalent settings in Xamarin Studio but it doesn't start anymore - even after repairing, un-installing and installing latest version from http://www.monodevelop.com/download/.

This are the settings exported from Visual Studio, mabye they can be imported into Xamarin Studio: Exported-2017-02-03.vssettings.zip

[wilhelmberg]

Another one.

I prefer

tile.transform.localPosition = new Vector3(
    tile.Rect.center.x - ReferenceTileRect.center.x,
    0,
    tile.Rect.center.y - ReferenceTileRect.center.y
^^^ just one tab
);

over

tile.transform.localPosition = new Vector3(tile.Rect.center.x - ReferenceTileRect.center.x, 
                                           0,
                                           tile.Rect.center.y - ReferenceTileRect.center.y);
                        ^^^^^ endless amount of spaces

[david-rhodes]

I prefer tabs over spaces. FWIW, when I change my C# formatting to Visual Studio style in Xamarin, it checks the convert tabs to spaces option box. Is that default in VS? In any case, I think we should use tabs.

I like private fields to have an underscore and start with a capital letter private int _Level;

I personally prefer lower case for private fields. The reason is that public modifiers should be used rarely, but Unity encourages developers to throw them around whenever they want something exposed in the inspector. This is wrong (and Unity will automatically properly name fields in the inspector if named _something, to `Something). Uppercase for anything that might be exposed to another object + methods, lowercase for anything private.

I like to have code that belongs together as compact as possible - opening brackets on same line and if it's a very short statement sometimes even everything on one line:

I don't mind having longer documents. Often, it's easier for me to spot matching braces when they are horizontally aligned (new line). I usually avoid simple one-liners like: set { this.zoom = value; } just to avoid formatting inconsistencies, but I won't fight to the death for this.

I prefer to have braces even for one liners. It's just so easy to overlook this when refactoring and adding a second statement:

So much yes!

tile.transform.localPosition = new Vector3(tile.Rect.center.x - ReferenceTileRect.center.x, 
                                           0,
                                           tile.Rect.center.y - ReferenceTileRect.center.y);
                        ^^^^^ endless amount of spaces

I usually let my IDE handle multi-line statements. I'm actually not sure how to change formatting rules related to this because it never bothered me enough to change them. If it starts looking funny, I just auto-format the document. How do you modify this setting in VS, @BergWerkGIS?

[wilhelmberg]

I prefer tabs over spaces. FWIW, when I change my C# formatting to Visual Studio style in Xamarin, it checks the convert tabs to spaces option box. Is that default in VS? In any case, I think we should use tabs.

🎉 tabs 🎉 Yes, spaces are default in VS.

I personally prefer lower case for private fields.

I'm ok with that. It's just when looking at longer methods that extend beyond the screen where I cannot see the variable definition I like to defer method or class scope from the name. Saves moving the mouse over the variable and checking the icon:

I don't mind having longer documents. Often, it's easier for me to spot matching braces when they are horizontally aligned (new line).

Yeah, virtual space is cheap. No problem with whatever we decide on, VS has nice highlighting, bracket matching and collapsing:

I usually avoid simple one-liners like: set { this.zoom = value; } just to avoid formatting inconsistencies

Me too, but I like to break my own rules sometimes - makes me feel like a revolutionary

I usually let my IDE handle multi-line statements.

Me too, Ctrl+k+d (format document) plus Ctrl+Shift+s (save all) happen automatically without thinking about it almost every few seconds.

How do you modify this setting in VS

I just put in a new line after new Vector3( and then the rest happens automagically.

[david-rhodes]

@BergWerkGIS Bringing this thread back to life since we will actually need to enforce these standards before open-sourcing (preferably). Honestly, this should be a trivial task and many of us can contribute in different areas simultaneously to split up the work.

http://wiki.unity3d.com/index.php/Csharp_Coding_Guidelines covers most of what we've described above.

I will add:

• I don't like comments--they are outdated quickly or are simply redundant. Instead, use good variable and method names. The exception is using comments for documentation purposes or complex algorithms.
• _variableName for privates (it just saves so much typing over thisand is commonly accepted in Unity development, at least).

/cc @isiyu because we discussed this during our planning session Friday.