Practical tips for clean C#

I obsess over keeping code as lean as possible and removing absolutely everything that doesn't have an explicit purpose. Having worked with C# for many years, I see some unfortunate patterns repeat themselves.

Yes, this is opinionated. Yes, there are always exceptions - especially when working with old code bases. Not all of these must necessarily be objectively correct in every situation. Just let things be concious, active choices - not accidental design.

Code is a liability. Code costs. Lifetime cost of code extends far, far beyond the initial write. The cost of not deleting old code is huge. The more code, the more space for issues to form.

There are many reasons to keep your code clean, like:

• IDE-performance. The developer experience impacts the end result. If something is too hard in a given context, it won't be done, or shortcuts will be taken. Less code means less stuff to analyse for your IDE.
• Less code, less complexity, less stuff means less cognitive load. This makes the code base easier to reason about and easier to learn. It reduces the  number of "What does this really do?" you ask yourself while working. When done right, it'll also increase cohesion, making it easier to build your own mental model of the code base.
• Dependency management and control. Being in control of what is being used where and when is a superpower that should not be under estimated. This aids you in avoiding accidental complexity and accidental dependencies.
• It could even aid you in keeping your deployment package small.

Don't abuse projects

I often see an large amount of projects in a solution. Generally, having tons of projects slows down your builds and IDE, and can make dependency resolution cumbersome. Upgrading NuGets may get tricky if you have 80 projects with explicit dependencies to everything everywhere. There's good reason the new SDK style project was introduced in the modern generation of .NET. Do take advantage of transient dependencies, and don't make new projects for everything. A good rule of  thumb is to keep everything in one single project until you hit a good reason not to. In many cases, simple feature folders folders/namespaces are better choices.

Good reasons to split code into a separate project/csproj:

• Distribution and deployment. If some code is going to be packed as a NuGet and shipped to external consumers or deployed somewhere. This might be packages with common functionality, or contract packages full of POCOs or similar shared resources.
• Development time dependencies. It's common to split unit tests and other non-production code into separate projects to avoid pulling in test infrastructure in production code, as it bloats your deployment bundle and generally increases risk of deploying things you really don't want in production. A good rule of thumb is to never reference test projects from other projects. For example, you don't want your mock classes being picked up by assembly scanning or your fancy plugin architecture in production.
• Implementation hiding. To abstract away and hide implementation details from other projects. Careful with this though, as this is dangerously easy to overcome in IDEs (a quick hotkey-push, and you've added a reference and using-statement, voiding the whole point). If everything is public anyways (as it too often is), you basically achieve nothing.
• Controlling dependency trees. In larger solutions, it may make sense to split out certain parts of the code base in a separate project. I'd say this only applies if you're in good control of the dependency graph, however, as it's still very easy to just ctrl+. or alt+enter your way around it, adding references to anything everywhere. Beware of the "onion architecture". It's more fluff than good.

Bad reasons to create a new project for your code:

• It uses a different technology than the other projects. Splitting code because of tech-dependencies only gets you in trouble in the long run. Split things in Orders and Customers, not EntityFramework and Dapper. And most likely, your domain entities don't need separate projects either.
• You feel like it should be a new project.
• You use it as top level folders, giving a rough baseline structure in your project. Use folders instead. It's lighter, more flexible in the long run and you avoid things not being possible in a year because of circular dependencies.
• Implementation hiding. Yep, it's also a bad reason, for the very reason listed above: In practice, it usually doesn't work, since it's all in one solution anyways.

Keep your .csproj-files clean

• Remove all unnecessary <PackageReference ...>. Let them come transiently through your other projects or referenced packages, if possible. The fewer explicit dependencies you have, the less of a chance of ending up in tricky updates with un-resolvable dependency trees without a whole load of manual intervention, and the faster it is to update your packages.
• Remove defaults. No need to specify an <AssemblyName> that exactly matches the file name of your csproj.
• Stuck on old style non-SDK-csproj? I feel sorry for you. Make sure you get time to upgrade as soon as you can.

Split by business domain, not by technology

Don't divide your code based on what ORM you use or whether it's a repository or an exception. Divide it by business domain concepts - features if you will. This means you should generally not have root-level folders/namespaces like EntityFramework, Mappers or Models. You should rather have things like Customer, Billing or Orders. Anything that belongs to the customer handling should be in that folder, not split in several other root folders based on what underlying framework is used. This:

• Increases cohesion, thereby making it easier to find related things, learn and reason about.
• In my experience, it reduces the feeling of having to split things into multiple projects, which in turn avoids other issues mentioned above.
• Makes it easier to implement changes or remove a feature.

Careful with static methods

Deep graphs of static methods work great until they suddenly don't. You may need to introduce something that just cannot be static. Then it's a total nightmare. Good rules of thumb for static methods (and most extension methods):

• Don't depend on things that aren't either inputs to the method or base types, like lists, DateTime, string etc.
• Don't nest static method calls.

One class, one interface

There's no reason a class cannot implement multiple interfaces. Too often, a strict 1:1 between classes and interfaces seem to happen.

Often, implementing multiple related interfaces in one class can make for writing less code, gathering related things. For example, there's often no reason to let ISomethingReadModel and ISomethingStorage be implemented in different classes.
You depend on the interface you need anyways, (usually) not the implementation, so you can still keep your calling code clean, whatever that means.

In my experience, implementing multiple related interfaces in a single class will:

• Again, increase cohesion, making it easier to find related things, learn and reason about.
• Reduce the sheer amount of code. Fewer files, less setup (for example in your IOC-container), often a smaller dependency graph, and potentially even a more restricted exposed API from your class. Which is good.

One interface, one implementation

Kindof related to the point above; with the whole 1:1 between interfaces and classes, you don't actually gain much from the interfaces. You could just as well remove them, and the whole code base would for all practical purposes be the exact same.

Have an actual reason to have that interface. Start off without one, add it when you need it:

• For testing purposes; faking out parts of the code becomes trivial.
• If you need multiple implementations.
• If you want to gather related concepts in a single class, but expose different APIs to different consumers.

Be explicit about everything

Be explicit about everything. Take advantage of all the static analysis modern IDEs can give you. Write plain, basic code, don't wrap everything in magic. "Find references" and "Find usages" are IDE-superpowers. The more magic reflection-based assembly-scanning frameworks you use, the worse all tooling works. By writing plain old C# code, you move runtime errors to compile-time, which is awesome. This is what the whole Javascript community writes hundreds of tools and frameworks to achieve. Don't use fancy libraries to take it all away from yourself.

• Don't try to make generic handlers to deal with all possible cases. The classic IRepository<T> is a horrible anti-pattern. Why would you expose an Update method for an object that should never be updated?
• Avoid things like AutoMapper and other reflection-based tools as much as possible. Being able to navigate with Find usages/references is gold.
• Most non-trivially sized solutions utilize IOC containers these days. As far as it's possible, add everything it should know about explicitly. Avoid assembly scanning if you can, or at least limit it tightly by namespace. This, again, aids your IDE and moves issues from runtime to compile time. It also gives you confidence in what's actually in there, giving you the desired control in case someone else screwed up and included a reference to your test project full of mocks somewhere.

Unnecessary hops

Methods that abstract away but bring nothing to the table. I see this too often, and it always strikes me - what problem does this actually solve? It brings no new semantics in - the date is already exposed in the public API, and it doesn't do any form of orchestration:

public void ImportOrders(DateTime date) 
{
    ImportOrdersForDate(date);
}

private void ImportOrdersForDate(DateTime date) 
{
    // Actual code
}

I even see this with whole classes sometimes, where there's no added anti corruption protection, no additional mapping, no orchestrating, no new semantics,  no added testability, no new nothing. It's just a thin middle layer that doesn't really contribute to the total picture.

public class SomethingManager 
{
    private ISomethingGateway _gateway;
    
    public SomethingManager(ISomethingGateway gateway) 
    {
        _gateway = gateway;
    }
    
    public Customer GetCustomer(Guid customerId) 
    {
        return _gateway.GetCustomer(customerId);
    }
}

I realise this can end up being what it is for historical reasons. But then it is your job to clean it up.

Overly defensive code

Given a request-object containing properties you need to look up something in a database, as the consumer of such an object, expect the format to be correct, and crash if it isn't. This is almost always the best way. Don't try to be defensive and return the "negative result", as this carries a different meaning to a failing one.

if (Guid.TryParse(request.ItemId, out var itemId)){
    return new Response { Item = _itemRepository.Get(itemId) };
}
else
    return new Response { Item = null }

This hides a bug. If the request doesn't even contain the correct kind of ID, that should simply not be the same as "We can't find what you look for".

The same with adding in "defensive measures" like the one I saw from Twitter a while back. The code may no longer crash, but it may also no longer be doing the correct thing. You decide what's worse. And then what's worse now, and what's worse in five years when your whole database is full of garbage data.