I have opinions!

I've been meaning to write this article for a while. I have various opinions about code formatting, and they may or may not conform to the norm. Also, they change occasionally as and when I get a good explanation of why a different way is better, or when I discover a technique while learning a language that I just have to incorporate into my C# code.

Firstly, let's just say that there are Guidelines and Opinions. I might not be clear exactly which one something is below, but it will be one or the other. That means that what I say won't fit all situations. Don't take it as a hard and fast rule. Use your own judgement.

Also, I love feedback, it is how I change and evolve my own style. I would love to revisit this in another couple of years and share how it might have changed. Please leave comments at the end.

One final thought before I start: Decide this as a team. If your whole team have individual styles and conventions, the code can feel like reading a book with multiple authors, disjointed and hard to follow. Just like a book, different chapters can have different authors and work well, the code can have different styles in different modules (libraries, packages, repositories). As long as the whole team is using the same style in the same code together, it will become more cohesive to read, write and maintain. And that is the ultimate point of applying Style Guidelines in the first place.

An Overview

Before I go into my styles, let's look at something more general, and standard.

Microsoft Guidance

Starting with something simple. If you are new to Microsoft Development and .Net, I would recommend reading their Naming Guidelines and General Naming Conventions. At the very least, it makes it easier to read other peoples code. If they write in C#, there is a high chance that they follow these guidelines. Either because they are part of a team that enforces them or the tooling uses it by default (Visual Studio, VSCode, Resharper, Rider etc) by their auto-formatter, or something like FxCop or StyleCop is enforcing it.

I may deviate a little, but this guidance was my starting point when I first began writing C# code.

Contracts: Public Vs Private

There are two places where you can apply your style and convention. Internally in your code, as you write it, and externally in the public API and contracts that your library exposes to other programs to use.

Before taking anything else into account, I recommend that any public signatures (e.g. naming conventions - class names, method names, parameter names, properties etc) Should follow the guidance from Microsoft. People using your library should not have to deal with your style and conventions, they should match what the expected C# library conventions are. This can also include protected methods and properties if your library is extensible.

Inside your code, its all up to you. Anything private (and protected if the class is sealed) is never exposed to the consumer, you can do what you like. Similarly, the layout of your code (whitespace, braces, class/file distribution) is all up to you (and your team).

Check Out M' Style

Let's get specific on how I do.

In general my rules are pretty hard to describe, so hopefully, the examples will help (I will use as the representative of space and as the representative of tab - SPOILER: I will never use this tab character...)

Naming Conventions

Every person and every language has their own approach to naming conventions. In C# there are a few basics: Capital letters starting Namespaces, Classes, Methods, and Properties and we use PascalCase by default, and not underscores. Properties do not start their name with "Get".

I stick to this as a strong rule. All methods, All Properties, anything public.

In general, variables and parameters use camelCase.

I use a couple of conventions for naming things well. For instance, I might name something that is a boolean IsOpen rather than Open or Opened, and for a setting CanOpen. Is and Can prefixes help readability of the code a lot. This is for properties but applies to methods just as much.

Private fields always (always) start with an underscore (_). When you are reading code, it makes it easier to determine what belongs to the method, and what belongs to the class.

Use long descriptive names, not single letter variables. Even if you are using Linq, if the name of the item in the lambda isn't obvious, still use a full name as the lambda parameter.

// bad
var names = data.Select(w => w.Name).ToList();

// better
var names = data.Select(widget => widget.Name).ToList();

Hungarian Notation

Related to naming things is Hungarian Notation. Now there are two interpretations to this, the wrong one, and the original one. Most people remember Visual Basic, and using btnSave and txtName for UI controls, or the terrible strName. This is the wrong interpretation of HN. Notation shouldn't be re-declaring the type which the reader can discover themselves, but instead to indicate the meaning of the data to avoid ambiguity. I think the name people use for the correct way is "Apps Hungarian".

Anyway, I use this when it is useful. My simplest example is when I have an argument that is an int providing a time. I will call my parameter timeoutInSeconds. Not strictly Hungarian Notation, but serves a similar purpose. Of course, if it makes more sense, I will use a TimeSpan instead, but sometimes an integer amount of seconds is a better API.

This also extends to units of measure. If we are using real world values we name the variables with their unit of measure. lengthInMeters and lengthInInches helps the readability of code, especially when you have to mix units. Also if your mixing amounts with volumes, it helps to have qtyAmount, mnyUnitCost and mnyTotal to identify which decimal is a quantity, and which is a monitary amount.

Or better yet, use Value Types for all of the above, instead.

Method signatures

If anything in this article will surprise you, it is probably this one.

I can't remember when I first saw this, but it took years to finally sink in and for me to grok it. It has to do with long signatures.

First, try not to have long signatures. decompose your classes to shrink their constructors, and use Parameter Objects to pass around instead.

Sometimes the problem isn't the number of arguments, but the names of those arguments. Nice long descriptive class names will quickly lead to this problem, even with only two or three parameters, especially Constructors when IOC is in play (which is always, right?).

public•class•MyClass
{
••••public•MyClass•(MyFirstDependentServiceWithALongName•myFirstDependentServiceWithALongName,•AnotherDependencyYouNeed•anotherDependencyYouNeed)
••••{
••••}

...

}

Is your screen wrapping, or is it just me?

There are two problems with this style.

  • You need to scroll your code window to see the whole signature, and when you scroll you lose the rest of your code. (I don't care how wide your screen is, this can and will still happen.)
  • Your source control tool with thrash on the signature line, with high potential for merge conflicts and such during development.

Like I say, it took me years after reading this idea before actually using it. Let it stew a bit in your mind.

public•class•MyClass
{
••••public•MyClass•(
••••••••MyFirstDependentServiceWithALongName•myFirstDependentServiceWithALongName,
••••••••AnotherDependencyYouNeed•anotherDependencyYouNeed)
••••{
••••}

...

}

We put each argument on a new line. They are indented an additional four spaces (the default indentation amount for one indent level). That last closing brace is on the same line as the last argument. (Because it will have to change to a , anyway if we adding another arg, and the function body braces work better with it that way.)

Always, always put the first argument on a new line. If you don't, the indentation is too wide, and if you rename the method, the indentation would keep shifting. You want to keep a stable, predictable indentation, within a method, and between methods. Consistency. (Again, this helps source control)

Let's see that without the visible whitespace:

public class MyClass
{
    public MyClass (
        MyFirstDependentServiceWithALongName myFirstDependentServiceWithALongName,
        AnotherDependencyYouNeed anotherDependencyYouNeed)
    {
    }

...

}

I find this style much easier to read, now.

It even works during the method calls:

var result = ProcessData(
    inputData,
    customers,
    authorisationData,
    filterOptions,
    sortOptions);

The main take-away for me is, follow the Yoda method. Do or do not.

Always put everything on one line, or always put all arguments on their own lines.

Braces

Braces go on new lines, not like JavaScript where they start on the same line as the header line. Pretty sure that is standard.

Always use braces when optional.

So there is some great guidance that if statements and similar should only ever have one statement, so the braces for a body block should never be necessary. That's fine. But the main reason for overriding it with my rule is the following. With no braces, you leave that code open to an edit vulnerability that can trick even the most experienced programmer to miss because our brains love to trick us.

if (someCondition(withAnArg))
    DoWork();
    
if (aDifferentCondition(wthAnotherArg))
    DoDifferentWork();
    ButAlsoDoAnotherThing();

That second example is lying to you.

look again with braces:

if (someCondition(withAnArg))
{
    DoWork();
}
    
if (aDifferentCondition(wthAnotherArg))
{
    DoDifferentWork();
}
    ButAlsoDoAnotherThing();

Even without fixing the whitespace, there is no mistaking what the second set of code does, whether you can read C# or not. It is far too easy for someone to start with code like the first if (someCondition(withAnArg)) and add another line that they think is inside the condition, but actually isn't, like what has happened to the if (aDifferentCondition(wthAnotherArg)) condition in the first example.

That is why I always enforce braces.

Lambdas are fine. you can leave out the braces on one-liner lambdas because the compiler will sort you out if you get that wrong.

General Whitespace

StyleCop has really helped me get stricter with my whitespace, enforcing my rules at compile time.

These are really minor things, but I find they aid readability.

  • Use a space before ( in method signatures (but not in calls)
  • (As Mentioned above) signatures on lines longer than 80 chars wide puts each arg on a new line.
  • If you have your curly braces ({ }) on one line, they should have spaces on either side.
  • between class names and the base-class/interface, one space either side of the colon (public class MyClass : IMyInterface)
  • spaces either side of lambda arrows (w => w.Name) (Also most symbols really)
  • One space after starting a comment line/block (This is law, John Skeet declared it so on a podcast)
  • Use 4 spaces for indentation
  • No tabs, ever
  • No spaces on empty lines (Just being anal)

Mostly, I just follow the Out-Of-The-Box settings from StyleCop.

Privates

As mentioned, I enforce underscore (_) prefixes for private fields. And I am not alone.

(stolen from the comment linked above:)

  • It eliminates conflicts with camel-cased parameter names - no need to use "this"
  • It's a visual indicator that the internal persistent state of the object is being read, or - more importantly - being written. It's a flag saying "this has side effects outside of the particular method I happen to be looking at", which is very important to know when looking at unfamiliar code.
  • Although you can apply the above as a reverse argument for this, the above will be enforced by the compiler non-optional, while this can be left off by choice.

Statics tend to vary a bit. If they are static and readonly, I have been known to use a starting Capital letter PascalCase. For consistency, often Mutable Statics are named the same.

I have been known to use ALLCAPS for Constants. This is a carry-over from C I suspect but makes it easier to spot a Constant in code. Sometimes this feels too harsh, and I may use PascalCase here instead. What is the difference to the end user between a private static readonly and a private const anyway?

My thinking is that if it starts with a Capital letter (PascalCase), it isn't owned by you. Either it belongs to the class you called to get it (property) or it doesn't belong to this instance (Static). For this thinking, I often will use Pascal Case for Protected fields as well, so that derived classes can tell they do not own it, either.

These rules are a bit less thought through, and I don't tend to be any stricter than my tooling tells me to be. (If Resharper or StyleCop says to change it, I usually will.)

Tests

This is probably the most controversial and the most varied. Somehow, when it comes to tests, it feels easier to break the rules.

Don't get me wrong, I subscribe to Uncle Bob, and your tests should be given the care and attention that any of your code is given. SOLID, DRY, Coupling vs cohesion, modularisation etc etc. But tests exercise requirements, so readability is more important than expected standards. Also, tests are all under your domain, your style can flourish, you are the only consumer.

What I am talking about primarily is using underscores in test names rather than strict PascalCase.

I have been experimenting, and I have two approaches I use:

  • Every_Word_Is_Seperated_By_An_Underscore_So_That_It_Forms_A_Descriptive_Sentence
  • GivenIHaveATestMethod_WhenINeedToNameIt_ThenISeperateTheGivenWhenThenComponentsByUnderscores

I have more recently extended my ideas to following this further guidance of organising tests to now use nested classes, and inheritance:

public class GivenIHaveAUser
{
    public class GivenIHaveLoggedIn : GivenIHaveAUser
    {
        public class GivenINavigateToTheFormPage : GivenIHaveLoggedIn
        {
            public class GivenISubmitAValidForm : GivenINavigateToTheFormPage
            {
                [Test]
                public void ItSubmitsSuccessfully()
                {
                   ...
                }
            }

            public class GivenISubmitAnInvalidForm : GivenINavigateToTheFormPage
            {
                [Test]
                public void ItShowsValidationMessages()
                {
                   ...
                }
            }
        }
    }
}

Automation

If you want to really care about the style of your code, I recommend installing a few NuGet packages into your projects to get compile-time enforcement (don't worry, the rules are configurable):

<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.6.0"  PrivateAssets="All" />
<PackageReference Include="codecracker.CSharp" Version="1.0.3" PrivateAssets="All" />
<PackageReference Include="StyleCop.Analyzers" Version="1.1.0-beta004" PrivateAssets="All" />

(That needs a whole article in itself, but this is a taster.)

The End

I hope you enjoyed reading and thinking about code formatting. If you haven't taken something away to try, read it again and find something to challenge me on. If you can't make an argument for or against, maybe give it a go yourself, or suggest it to your team.