Today I will be presenting my ideas based on the analysis of the code issues I found while re-factoring. Some of the issues I have found are not too serious, but there are many areas where it could be done better (much better) So here are the notes:
* Solution has to build. It might seem obvious but the developers here don’t follow one of the most important rule. When checking in the code into the branch, especially if there is more than one person working on it, it has to build! So many times I come in in the morning and seeing that someone checked in the code that doesn’t compile and that in turns means that I cannot do much until that is fixed.
Rule #1 – the code you check in must compile, full stop!
* Tuples – I was never in favour of tuples, but they have their use, but that use should be constrained to internal implementation. Tuples should never be exposed in a public interface to prevent other developers working on the code guessing what Item1, Item2 …Itemn stand for.
Rule #2 – do not expose tuples in public interfaces, use a class or a struct, which ever one is more appropriate.
* Singletons and Static members – don’t use them. If you need a singleton like behavior do it using you preferred IoC container. There is a valid case for using static methods, if the method is stateless you should declare it as static, but that is different.
Rule #3 – Do not use static members and singletons.
* Separation of Concerns – don’t create classes that do ‘everything’, don’t create a facade to a database. Instead split the class into smaller chunks each responsible to do one thing and do it well. That simplifies development and lets you to trace what is being used and where. It quite often that you would then realise that not everything is used and some code can be removed.
Rule #4 – Split large classes into smaller ones.
* Exception Handling – always catch those exception that can be handled. There are valid reasons for catching all exceptions, however before doing that, put catch statements for exceptions that you can do something about. The generic catch statement then can just be used for logging
Rule #5 – Catch the most specific exceptions first and generic exceptions last.
* Dispose and Finally – You should always implement IDisposable if any of the fields are disposable. Think hard before implementing a finiliser. When objects with finilisers are instantiated they are placed into the finalisation queue. While it is fine when the application is single threaded it could create contention to finilisation queue when instatiating those objects from multiple thread. But if you doing that it is a definite sign that you’re doing something wrong.
Rule #6 – Implement IDisposable if any of the fields is IDisposable. Use finalisers with caution