Be more productive with fewer keystrokes: Access modifiers

This is the first post of many giving a summary of my experiences with regards to whether I found typing more or typing less to be more productive in certain situations.
Typing less means reading less, which is good. But sometimes expressing things more verbosely or using language constructs that use more characters initially will pay for itself in the long-term, e. g. by making better use of the compiler or improving code navigation and debugging.

Code is littered with unnecessary access modifiers

This first post is about access modifiers.

TL;DR: Only declare access modifiers if you do not want the default. This makes the code much more readable and succinct.

The majority of projects I have seen explicitly declare access modifiers on everything on which they can be declared. This is Visual Studio’s default when generating members, too. ReSharper, FxCop and even the Roslyn analyzers giving you IDE0040 will by default tell you to always declare access modifiers.

I do not endorse this practice.

It leads to code where every private method has a private modifier. So does every member (private readonly anyone?). That is a lot of privates (pun intended).
It also leads to developers — forced to type something — making classes and interfaces public by habit, even in those cases where internal would have sufficed and should have been preferred as the lowest viable visibility.
This in turn leads to actual public-by-deliberate-design classes and interfaces not standing out anymore. The public API of the assembly becomes unnecessarily large.

Trust the defaults

The C# language has defined sensible default access modifiers for the different language elements. They reflect the lowest sensible visibility: non-nested classes and interfaces are internal, nested classes are private, members are private. This is what is needed most of the time anyway. I see no benefit in explicitly specifying default access modifiers. On the contrary, they add a lot of noise and words the reader needs to mentally skip when reading the code.

Compare how succinct this reads:

class Foo
{
    readonly FirstDoer _firstDoer;
    readonly SecondDoer _secondDoer;

    public Foo(FirstDoer firstDoer, SecondDoer secondDoer)
    {
       _firstDoer = firstDoer;
       _secondDoer = secondDoer;
    }

    public void DoStuff()
    {
        DoFirst();
        DoSecond();
    }

    void DoFirst()
    {
        _firstDoer.Do();
    }

    void DoSecond()
    {
        _secondDoer.Do();
    }
}

in comparison to this:

public class Foo
{
    private readonly FirstDoer _firstDoer;
    private readonly SecondDoer _secondDoer;

    public Foo(FirstDoer firstDoer, SecondDoer secondDoer)
    {
       _firstDoer = firstDoer;
       _secondDoer = secondDoer;
    }

    public void DoStuff()
    {
        DoFirst();
        DoSecond();
    }

    private void DoFirst()
    {
        _firstDoer.Do();
    }

    private void DoSecond()
    {
        _secondDoer.Do();
    }
}

Extra points if while reading you tripped over public class and asked yourself why this is public all of a sudden. For a lot of people the public does not even register as anything noteworthy anymore.

I much prefer the first version.

Choose your style rules wisely

If you feel the same way, disable all rules enforcing default access modifiers and configure that IDE0040 with dotnet_style_require_accessibility_modifiers = omit_if_default in your .editorconfig. AFAIK, Visual Studio will not honor those rules when generating members though, so you need to remove the access modifiers on those manually. If you set your style rule diagnostics to warning level, you will at least be helpfully reminded to remove the access modifiers if they are default.

What about the unit tests?

If you follow my approach, you will end up with more internal classes and interfaces, and rightfully so. This leaves the question of how to unit-test those, should it be necessary. Making them public again would muddle your design.

In contrast, making the implementation projects InternalsVisibleTo the unit test projects to test internal classes is risk-free and often needs to be done anyway. If you use DynamicProxy-based mocking frameworks like Moq or NSubstitute, don’t forget to make the internals visible to DynamicProxyGenAssembly2 , too, while you are at it.