When starting new projects, one of the first things to do, especially with a new team, is to agree how the code should be written. Writing code is a process, a part of the wider process of shipping valued software, and so any advice has to cover the process of coding, not just the output itself. This is one of the reasons that I don’t find value particularly in ‘coding standards’ documents. It’s not that these things aren’t useful, it’s just that their use is quite small.
There are many great books written on this subject, and for me it’s important that developers read these and discuss what they think. Good code is written by thinking developers, developers who are reflective practitioners.
I’ve purposely left out in what follows popular acronyms like DRY, SOLID, CQS, DI, etc…, but they’re definitely echoed.
What follows has a C# bias, but I think it has a general applicability well beyond that (simply substitute Resharper for another similar tool, or NUnit for JUnit). I might be accused of being a software conservative from reading it. That would probably be right. I don’t think that’s a bad thing. C# as a language has been designed with that bias, to buck it, to go against its grain, is to seek problems. Besides that, I think to prefer the explicit is a good thing. I think type systems are a good thing. I think …
So…
- Use your professional judgement. As a professional you should have, develop, and apply your judgement. Ultimately that’s the hard bit of being a coder. If you don’t use your judgement, you won’t develop it. Recognise though the weaknesses in your ability to judge and seek the counsel of others, none of us are perfect, and none of us are right all of the time.
- Be consistent. Preferably be consistent with others. More preferably still be consistent in making the right choices. If in doubt about what the right choice is, ask a colleague, if they don’t know ask another colleague. If a colleague is very sure, challenge their view – very little is black and white. Don’t spend all day doing this, balance the need to maintain momentum.
- A Resharper settings file should be shared across the team (solution team shared). Always save Resharper settings to this. Before changing settings, check with your team.
- Listen to Resharper – generally, it is right and you are wrong. If you are confident you are right, check with a colleague. Try to ensure that there are no Resharper warnings present in the file – Resharper should show your code as ‘green’. If Resharper is wrong, add a comment to explain why. If the thing Resharper is warning about is not important then check with your team, and if they agree, alter the settings to mute the warning (perhaps making it just a hint) and save this setting to the team shared Resharper settings file.
- If adding comments (including TODO comments) to the code, also add your name, or an abbreviation of it that will be recognised (I typically use first letter of my first name followed by the first two letters of my surname, eg NRO for Neil Robbins). It’s very useful to know who made a comment so it can be followed up in person if helpful.
- If a method can be static, then make it static. Not marking a method that could be static as static does not make it better. Then consider whether it belongs in the class it is in. Generally speaking, static functions belong in static classes, mixing static and non-static methods in the same class suggests a lack of internal cohesion to the class.
- Methods which return state (non void methods) should not mutate observable state (so, its ok to log the call, but not to change some state in a way that effects future calls to this or other methods). Methods which mutate observable state should not return anything (they should be void). This is known as the Command Query Separation principle. It is fundamental to ensuring code can easily be reasoned about. Code should not have surprising effects. Code should have a rather dull, Ronseal like quality. Code which cannot be easily reasoned about (by someone other than you) makes a project more expensive.
- Understand the code you are writing, and the tools, libraries, frameworks, etc… you are using. If you don’t understand these things then either gain that understanding, or don’t use them. Think about the code you are writing and its effects
- computational (does it produce a correct result, does it do this efficiently)
- business (does it meet the need of the business)
- social (other people who will need to understand it).
- Names should be descriptive of purpose/role. Don’t be afraid of long names, auto-completion will save your fingers from caring. Don’t use abbreviations or acronyms without very good reason, unless they are totally ubiquitous (like Id for Identifier).
- When naming things try to stick to idiomatic practices. If resharper warns you about your naming (eg you called something userID instead of userId), you’re not being idiomatic.
- Interfaces represent roles, things that objects can do. Separate roles demand separate interfaces. Try to keep the number of methods exposed by your interfaces minimal.
- Think about how you will test your code. Even if you’re not doing a test first style of development, consider how you or others will be able to test it. Testing things typically requires the thing being tested to be isolatable from its dependencies. This encourages loose coupling. This keeps the codebase flexible. A good thing.
- Keep your classes small. I use as a general rule, if your class has more than 100 lines, it’s too big. If the class inherits from another class, include the number of lines in each class in the hierarchy in this count.
- Keep your methods small. If a class should only do one thing, then that applies at least as much to a method. Composition starts with the composing of appropriately named functions.
- Prefer composition over inheritance. This piece of advice is the best thing by far in the Gang of Four patterns book. Inheriting behaviour is to be questioned. Providing protected ‘utility’ functions in base classes is an anti-pattern. This isn’t to say that inheritance is bad, it’s not, it is a very powerful, useful thing. It’s also massively abused. Prefer composition through the passing of collaborators to the objects/functions that require them.
- Seek internal cohesion. Ideally, every method in a class would use all of that classes fields. If you find that one subset of methods uses one subset of fields, and another subset uses another subset of fields, then your class lacks internal cohesion. Extract the subsets creating classes out of them. An object should do one thing only, that is serve only one purpose.
- Couple to interfaces for services. Function parameters that are purely data can be classes, but parameters which provide services should be specified in terms of interfaces. As mentioned earlier, these interfaces represent roles, so the function (and it could be the constructor) is specifying the roles that it requires collaborators to fulfil.
- Pass in collaborators, don’t construct them within functions. Preferably this will be through the constructor, if that is not possible then pass them into the function itself. Avoid the use of properties to provide collaborators.
- Don’t abstract too early. When is too early? It’s hard to tell, but wait until the last responsible moment, which is to say, until not having the abstraction is patently absurd/costly. Do abstract though. A nice maxim is, do it once, do it twice, refactor and abstract. Which is to say don’t be afraid to write essentially the same code three times before recognising and harvesting the commonality. This may seem like waste, but the cost of going down proverbial rabbit holes with early inappropriate abstractions tends to be far higher.
- Create types to express concepts, even if that means you have a lot more types. If you capture a suppliers name, create a type called SuppliersName, don’t just use a string. It takes seconds, but makes a codebase a lot more readable. Use implicit operators to provide for transparent casting back to a string, or from a string, if that makes it simpler to use when persisting to a database or similar. This is the closest I’ve found you can get to aliasing types like you can in languages like F#. Once you have this concept in your codebase you may find that you start to find behaviour that should belong to it. This applies also to collection types. Creating custom collection types that hide the data structure which they use internally, can provide far richer semantics making code much easier to understand, and giving behaviours a single, sensible, home. We’re paid to write code, don’t be afraid to write it. Less code is not always a good thing (cough APL – life←{↑1 ⍵∨.∧3 4=+/,¯1 0 1∘.⊖¯1 0 1∘.⌽⊂⍵} ). Making things explicit always is.
- Use Ruby/sentence style naming for tests (eg when_something_happens given_a_context_then_something_is_observed). Don’t feel required to use BDD style semantics. Use the semantic that best describes what you are testing (as with any method). A separation between the fundamentals of setting up the context, performing the action being tested, and examining the result to see if it is within the bounds of correctness, is pretty fundamental and useful however.
- Use plain-old nunit (assuming C#). It does the job plenty well. Most other C# testing libraries are overly complicated opinionated guff, or just a lesser copy of nunit. So, no Specflow, mspec, Cucumber, Gherkin, MSTest, MBUnit, etc… If Nunit seems to lack a killer feature of another library, you almost certainly don’t need it, and probably do need to simplify what you’re doing. If you’re lucky enough to work with stakeholders who are happy to sit with you and co-own tests that conform to the Gherkin DSL then congratulations, you are the <1% and using a Cucumber clone (or Cucumber) might benefit you. For everyone else it won’t. It’ll just add extra cost to what you’re doing. If you need libraries like Selenium that drive applications then great, use it, just use it with Nunit.
- If an action being tested requires a number of separate assertions consider creating each assertion as a separate method, with the action itself and the context it requires being carried out in a separate method, and only once for the fixture (use the TestFixtureSetup attribute). Name the assertion methods appropriately. Consider moving any complex logic required for the assertion into separate functions. I often use extension methods for this, or hand crafted mocks or spys.
- Have as few Projects in your Solution as reasonable. More projects == slower compilation. A project produces a .dll which is a unit of deployment/distribution. If the code needs to be deployed/distributed differently to the way the other projects are, then you need a new project. If not, then you probably don’t. One test project is usually plenty. Use test categories to mark different types of tests (the Category attribute of Nunit), don’t create extra projects.
- Not all external code (including a lot from Microsoft – cough their ASP.NET and ASP.NET MVC stacks) supports good practice. Consider using gateway code, that is, writing adaptors and façades. This decouples your code from the externally owned code and can keep the issues in the external code from bleeding into yours. It can also help to keep your code reasonably testable.
- Mocking is there to provide test doubles (classes that take the place of ones used in production) which are capable of verifying their use in a context. If you don’t need to verify anything then don’t need a mock, you just need to provide a data structure. If interfaces are properly defined and properly narrow then this shouldn’t be a problem. If they aren’t then change your code so that they are.
- Don’t mock types that you don’t own. This advice, at least popularised by the people who created the idea of mocking and the first mocking library, is good advice. A lot of the time, like with Microsoft frameworks, you won’t even be able to mock it anyway without resorting to ‘powerful’ mocking tools. These powerful mocking tools are unnecessary, because you shouldn’t be mocking anything you don’t own.
- Not all code needs to be of an equal standard. Some code can be pretty shocking and it won’t matter so much. Where it costs no more to get it right than to write crappy code (which is most of the time I find) try to think about the code you’re writing and write the better code.
Semeosis 
Articles like this one should be put in a golden frame. Thanks for your advices. If only every C# developer could follow it, things would be a lot different.
Thanks for your encouragement, much appreciated!
[…] https://blog.semeosis.com/2013/01/12/writing-code/ […]