Beginner programmers always spend a lot of time on learning a programming language, code syntax, technology and tools. They think, if  they master the craft of technologies, they will become good programmers. However, object programming is not about mastering the tools, it is about creating a solution to a problem in a particular domain and to do it in cooperation with other programmers. Therefore, it is very important to express your thoughts in code precisely and in a result to be understood by other people.

Let’s start with a great quote by the Clean Code guru, Robert C. Martin:

“The proper use of comments is to compensate for our failure to express our self in code”

[„Clean Code: A Handbook of Agile Software Craftsmanship” Robert C. Martin]

This phrase simply means that if there is a need to comment your code, it is most probable that your code is bad. Also, it might indicate a failure when you cannot express all your thoughts about a problem or an algorithm within code without comments. And finally, that means you expressed a part of a concept in comment not in the commented code. Good code should be understandable to everyone without reading any comments. Good coding style is about storing all necessary information to understand the problem in code.

In programming theory, there is a concept of “self-describing source code”. It is a common descriptor for source code that follows certain loosely defined conventions for naming and structure. The main objective for self-describing is to make source code easier to read and understand. Therefore, it is easier to maintain or extend existing code.

Within the scope of this article I would like to present some examples of “bad code” compared with “clean code”.

Names have to reveal your intentions

Thinking about names is always a problem when writing code. Some programmers are trying to simplify, shorten or encode names in they-only-know way. Let’s have a look at a few examples:

int d;
// elapsed time in days
int ds;
int dsm;
int faid;

Name “d” could mean anything. The author used comment to reveal his intentions, instead of including it in code. Name “faid” could be mistaken for identity (ID).

int elapsedTimeInDays;
int daysSinceCreation;
int daysSinceModification;
int fileAgeInDays;

Avoid Disinformation

No information at all is better than misleading information. Sometimes programmers try to “hide” some important information, however they tend also to create confusing parts of code sometimes.

Customer[] customerList;
Table theTable;

Variable “customerList” is not actually a list. It is a normal array (or just a collection of customers). In the second case, “theTable” is an object with a type “Table” (which you can easily check when using IDE), and the word “the” is just an unnecessary noise.

Customer[] customers;
Table customers;

Good names length

In modern programming languages, long variable names are not a problem. You can create names almost without any limitations. Nevertheless the problem is that this could introduce a naming chaos in code.

var theCustomersListWithAllCustomersIncludedWithoutFilter;
var list;

A good name contains as many words as are needed to express a concept. But nothing more. Any unnecessary words make the name longer and harder to understand. Short names are good only when they describe the whole concept in the current context (it is better to say “customersInOrder” than “list” in a context of making an order).

var allCustomers;
var customersInOrder;

Always code in one notation, let notation help you understand the code

Any programming technology (language) has its own “style”, called notation. A programmer should create code that matches this notation, because other programmers probably know it and use it. Let’s have a look at a bad example of code without proper notation. The code below does not fit any “standard” well-known notation (like PascalCase, camelCase, Hungarian Notation). Moreover there is a meaningless name for bool (“change”). This is a verb (describes action), but the bool value in this case describes a state, so it is better to use an adjective-like form there.

const int maxcount = 1
bool change = true
public interface Repository
private string NAME
public class personaddress
void getallorders()

When you look at a part of code, you should know straight away what kind of element in object programming it is, just because of the notation.

For example: you see “_name” and you already know that it is a private variable in the current class. You have to always use notation, without any exception.

const int MAXCOUNT = 1
bool isChanged = true
public interface IRepository
private string _name
public class PersonAddress
void GetAllOrders()

Use one word per one concept. Don’t mix multiple concepts per one word

Defining concepts is always a problem. In software development process a lot of time is spend on analysis of a domain and proper naming of all elements. Hence, programmers also have difficulties with concepts.

//1.
void LoadSingleData()
void FetchDataFiltered()
Void GetAllData()
//2.
void SetDataToView();
void SetObjectValue(int value)

First case:

The author of the code tried to express a concept “get the data”, using multiple words “load”, “fetch”, “get”. Only one word per concept should be used in code (in a particular domain).

Second case:

A word “set” is used for 2 concepts: the first is “data loading to view”, and the second is “setting a value of object”. These concepts are not the same, so you should use different words for each one.

//1.
void GetSingleData()
void GetDataFiltered()
Void GetAllData()
//2.
void LoadDataToView();
void SetObjectValue(int value)

Use meaningful names in domain context

All code that programmers write is connected to some domain logic. To make code more understandable to anyone involved in solving a problem, it is better to use meaningful names in a domain context.

public class EntitiesRelation
{
Entity o1;
Entity o2;
}

When you are coding domain-specific solution, you should always use domain-specific names. In the future somebody else (not only a programmer, maybe a tester) will use your code and will be able to easily understand it in a domain context (with business logic knowledge). You should think first about the domain problem, later about how the solution is going to be implemented.

public class ProductWithCategory
{
Entity product;
Entity category;
}

Use meaningful names in their self context

Apart from the element name in code, there is always some context that the name is used within. The context is very important to understand a name, because it has additional information. Let’s have a look at a typical “address” context:

string addressCity;
string addressHomeNumber;
string addressPostCode;

In almost all domains, a phrase “Post Code” is a part of address and it is obvious that post code cannot exists alone (unless you are developing a post code only application). So, it is unnecessary to add “address” as a part of the name. Moreover, all the information that is connected with a variable is contained in: a variable name, a class that includes this variable and a namespace that includes the class.

In object-oriented programming the best way is to design a class that represents an entity “Address”.

class Address
{
string city;
string homeNumber;
string postCode;
}

Summary

To sum up, as a programmer you should:

• Always try to express a concept when naming any element;
• Think about the length of names, they should contain only information necessary to understand intentions;
• Notation helps to understand the code, so use it;
• Do not mix names for concepts;
• Let the names be meaningful in domain context and their self context.

Feel free to share your thoughts about this issue. If you know any other problems with naming or expressing thoughts in code that developers have, please let me know in comments below

Dependency Injection – everybody’s doing it! It helps you write flexible, loosely coupled, reusable, testable and maintainable code. But fancy keywords aside, are you sure you know how to use it to reap all the benefits? Let’s have a look at a couple of tips on how you can use DI to help you write high quality code.

Always default to Constructor Injection

DI comes in many flavors, including:

• Constructor Injection,
• Property Injection (aka Setter Injection),
• Interface Injection.

All of them are tasty, but Constructor Injection is by far the most delicious. Why? Because it makes your class’s required dependencies explicit and ensures that they’re always available to your service. It’s also really easy to implement and as a bonus it can make your code more testable.

Let’s have a look at a code sample:

public class Service : IService
{
    private readonly IDependency dependency;

    public Service(IDependency dependency)
    {
        if (dependency == null)
        {
            throw new ArgumentNullException("dependency");
        }

        this.dependency = dependency;
    }

    //Service implementation...
}

As you can see all of the dependencies of the Service class are explicitly stated in the class’s constructor and can be easily injected by our IoC container. Furthermore, having all of the dependencies in the constructor guarantees their availability to the service – there is no way to create an instance of Service without supplying something implementing IDependency. The null guards in the constructor’s code are an additional measure to ensure that the dependencies of the Service class are always satisfied.

Constructor Injection has another advantage – it improves testability. In our unit tests we can now simply create a mock object for IDependency, construct an instance of the Service class and test it without worrying about what the actual implementation of IDependency does.

Remember your three R’s

The three R’s are:

• Register,
• Resolve,
• Release.

Also known as the Three Calls Pattern, the mnemonic describes the sequence of mandatory steps that accompany the usage of an IoC container. Here’s the kicker: each of those steps should be performed only once in the entire application. Impossible? Most frameworks actually enable you to do that! Let’s examine the three R’s rule based on the example of an ASP.NET MVC application which uses Castle Windsor as an IoC container.

Register is the first step in the sequence. This is the phase in which you create an instance of the IoC container and register your components with it. Registration should be performed in the application’s entry point. In the case of our MVC application the ideal place for this is the Application_Start method in the Global.asax file.

protected void Application_Start()
{
    var container = new WindsorContainer();
    //Castle Windsor uses Installers for component registrations
    container.Install(new MyComponentsInstaller());
}

The next step, Resolve, is the place where you construct your object graph. This should be done in your application’s Composition Root. A good candidate for a Composition Root in an MVC application is the place where our controllers are created – a controller factory. Our controller factory class should inherit from DefaultControllerFactory and override the GetControllerInstance method.

protected override IController GetControllerInstance(
    RequestContext requestContext, Type controllerType)
{
    return (IController)this.container.Resolve(controllerType);
}

How did the container get there? It was passed as a parameter the controller factory’s constructor.

Release is the third and final step and coincidentally the one which is most frequently forgotten. In this phase, components that are no longer needed should be decommissioned by the container. In an MVC application this can be done in the ReleaseController method of our, already implemented, custom controller factory.

public override void ReleaseController(IController controller)
{
    this.container.Release(controller);
}

Let’s sum up the whole example. Register? Check! Resolve? Check! Release? Check! Each invoked only once? Check! Having a single instance of an IoC container in the application and only three places where it’s used sure sounds like maintainability heaven, doesn’t it?

Don’t use the IoC container as a Service Locator

The infamous Service Locator is widely considered as an anti-pattern. That goes double for trying to use an IoC container as one. Why? Because it makes you lose most of the benefits of DI and more importantly it goes against the inversion of control paradigm.

An IoC container is used as a Service Locator when the code contains many calls like this:

public class Service
{
    //somewhere in a method, constructor or field initializer
    IDependency dependency = ServiceLocator.Resolve<IDependency>();
}

In the code sample above ServiceLocator denotes some kind of wrapper, static or not, on the IoC container. That’s bad due to several reasons. First of all, it introduces a hard dependency on ServiceLocator. Now, everywhere we use the Service class we have to drag the locator along with it.

Secondly, the dependency is implicit and even if we make it explicit (for example by making ServiceLocator, or even its abstraction, a constructor parameter), the dependencies the Service Locator provides will remain implicit.

Thirdly, how are we going to test our service? It looks like we have to use ServiceLocator in our tests. This hurts our testability.

My suggestion is: save yourself the trouble, switch to Constructor Injection, follow the three R’s rule and make your container transparent to the application you’re writing.

Use convention-based registration to your advantage

DI containers usually support three forms of component registration:

• XML-based,
• Code-based,
• Convention-based.

The first two options enable you to explicitly register components one by one. One of them uses XML files and the the other uses code. But the real power lies within the third form, which allows you to establish rules used for locating your application’s components.

Let’s have a look at a sample showing how we can register components using Castle Windsor’s convention-based registration API:

public void Install(IWindsorContainer container, IConfigurationStore store)
{
    container.Register(
        Classes.FromAssemblyContaining<IService>()
            .Where(type => type.Name.EndsWith("Service"))
            .WithServiceDefaultInterfaces()
            .LifestyleTransient());
}

The code establishes a convention that can be summed up as “Register all types which names end with ‘Service’ and which are defined in the same assembly as IService. Use the default interface and a transient lifestyle for the types”. I won’t explain the Windsor-specific jargon but this simple example should give you an idea of how powerful this API is.

What are the advantages of such an approach? Well, the obvious one is that you don’t have to repeatedly write registration code for each new type you introduce, you only need to make sure to follow conventions. This makes the registration code more compact and DRY!

Furthermore, you get the benefits of Convention over Configuration, namely: code consistency, reduced complexity and increased developer productivity.

Additionally, some advanced convention code can even give you the ability to interchange components during runtime. How cool is that?

Dependency Injection is a really powerful mechanism which can greatly improve code quality in more than one way. Has DI helped you achieve that goal? Do you have any other tips on using IoC containers? Make sure to leave a comment.

1. Try to avoid using the “ref” and “out” keywords.

2. Do not use OrderBy before Where

var allCitiesInPoland =
    allCitiesInTheWorld.OrderBy(x => x.Name)
                       .Where(x => x.Country == Country.Poland);

var allCitiesInPoland =
    allCitiesInTheWorld.Where(x => x.Country == Country.Poland)
                       .OrderBy(x => x.Name);

3. Always use Properties instead of public variables

4. Take advantage of string.IsNullOrEmpty() and string.IsNullOrWhiteSpace()

if (name != null && name != string.Empty)
{
    [...]
}

if (string.IsNullOrEmpty(name))
{
    [...]
}

if (string.IsNullOrEmpty(name.Trim()))
{
    [...]
}

if (string.IsNullOrWhiteSpace(name))
{
    [...]
}

5. Understand the difference between First() and Single()

6. Do not blindly use List

    class Program
    {
        static void Main(string[] args)
        {
            const int COUNT = 100000;
            HashSet<int> hashSetOfInts = new HashSet<int>();
            Stopwatch stopWatch = new Stopwatch();
            for (int i = 0; i < COUNT; i++)
            {
                hashSetOfInts.Add(i);
            }

            stopWatch.Start();
            for (int i = 0; i < COUNT; i++)
            {
                hashSetOfInts.Contains(i);
            }
            stopWatch.Stop();

            Console.WriteLine(stopWatch.Elapsed);

            stopWatch.Reset(); 
            List<int> listOfInts = new List<int>();
            for (int i = 0; i < COUNT; i++)
            {
                listOfInts.Add(i);
            }

            stopWatch.Start();
            for (int i = 0; i < COUNT; i++)
            {
                listOfInts.Contains(i);
            }
            stopWatch.Stop();

            Console.WriteLine(stopWatch.Elapsed);
            Console.Read();
        }
    }

Of course HashSets or SortedSets are not golden means for every possible scenario. HashSet should be used in case when you care about the performance (especially if you know that you will operate on a large set of items) but do not care about the order. SortedSet is a bit slower than HashSet but will give you a sorted collection and is much faster than List.

Use List when you want to iterate through the collection. Iterating through all the items in a List it is generally much faster than through a set (unless you use inside such methods as Contains).

If you know any other thing that C# developers should or should not do feel free to share it with us and leave a comment below!

P.S. What you should be aware of is that the guidelines presented in this (and in the previous post) will not suddenly make you a great C# programmer. What makes someone a good developer is the everyday compliance with the basic principles of creating high quality code which I described in details in the post Top 9 qualities of clean code.

How often do you express your disbelief when browsing through someone’s code saying out loud “Omg, that’s real spaghetti code…” ? Probably quite often. And how sure are you that no one thought the same when working with your code? In other words, how sure are you that your code is clean? The truth is that you can only be sure if you fully know what clean code means.

It is hard to create a precise definition of clean code and probably there are as many definitions as developers. However, some principles that lead to a basic level of clean code apply. I have gathered the 9 most relevant ones and described them in short below.

1. Bad code does too much – Clean code is focused

Each class, method or any other entity should remain undisturbed. It should conform to SRP (Single Responsibility Principle). Shortly speaking, we can say that SRP (according to some well-known definitions) is about making sure that if you can think of the reason for changing a class you should not be able to come up with more than one.

Though, I wouldn’t limit this definition only to the concept of classes. In his latest article Ralf Westphal presents a broader definition of Single Responsibility Principle. According to his definition:

A functional unit on a given level of abstraction should only be responsible for a single aspect of a system’s requirements. An aspect of requirements is a trait or property of requirements, which can change independently of other aspects.

If you would like to read more about the quoted thesis I advise you to dig into his article.

2.  The language you wrote your code with should look like it was made for the problem

It is not the language that makes a program look simple, but the programmer who makes the language appear simple.

(quote from Robert C. Martin)

This means, for instance, that you shouldn’t use workarounds which make code and language usually look awkward. If you claim that something can only be done by means of a workaround, it usually means that you haven’t spent enough time on trying to find a good, clean solution.

3. It should not be redundant

It should comply with the DRY rule (Don’t Repeat Yourself). When the DRY principle has successfully been applied, the modification of any single element of a system doesn’t require a change in any other logically unrelated elements.

4. Reading your code should be pleasant

When  browsing through the code you should feel like reading Harry Potter (yeah I know that’s a slight exaggeration ). You should feel that it was made to be read by any developer easily without hours spent on digging into it.

To achieve this you should try to comply with the KISS principle (Keep It Simple, Stupid!) and YAGNI principle (You Ain’t Gonna Need It). The KISS principle states that most systems work best if they are kept simple rather than made complex.

Therefore, simplicity should be a key goal in design, and unnecessary complexity should be avoided. YAGNI is a practice encouraging to purely focus on the simplest things that make your software work.

5. Can be easily extended by any other developer

You don’t write code for yourself , or worse -  for a compiler. You write code for other developers. Don’t be selfish – think about the others. Don’t torture other developers by producing a hardly maintainable and extendable code. Besides, in some months time you could be that “other developer” yourself.

6. It should have minimal dependencies

The more dependencies it has, the harder it is to maintain and change it in the future. You can always help yourself in achieving the goal of having minimal dependencies by using e.g. NDEPEND for checking potential incorrectness in the dependencies of your code.

7. Smaller is better

Code should be minimal. Both classes and methods should be short, preferably just a few lines of code. It should be well divided (also within one class). The better you divide your code the easier it becomes to read it. This principle might positively influence point 4 – it will make it easier for other developers to understand your code.

8. It should have unit and acceptance tests

How can we know that our code complies with the requirements if we don’t write tests? How can we maintain and extend it without the fear that it will stop working? Code without tests is simply not clean. If you would like to get to know more about the pillars of unit testing I advise you to read a very nice article Three Pillars of Unit Tests written by one of my colleagues.

9. It should be expressive

Expressiveness of the code means that it has meaningful names. These names should express their intention. They should not mislead you. They should be distinctive. Expressiveness makes code document itself making the need for documentation less important. If you want to read more about the subject of self-documenting code I recommend you to go through this article.

So what is in fact the definition of clean code?

All in all there is one final quality that summarizes all the above:

Clean code is a code that is written by someone who cares

 quote from Michael Feathers

It is written by someone who has treated it as an art and paid attention to all details.

The subject of clean code is in fact very complex and goes far beyond the knowledge presented in this post. Therefore if you find any other characteristics that you think make code cleaner do not hesitate and share them with us!

While working with (young) C# programmers I’ve noticed that some mistakes are being repeated by almost every one of them. These are mostly the mistakes, which once you point them, are quite easy to remember. However, if a developer is not aware of them, they can cause many problems with the efficiency and quality of the developed software. Therefore, I decided to gather the 8 most common mistakes made.

1. String concatenation instead of StringBuilder

String concatenation works in such a way that every time when you add something to a string, a new address in the memory is being allocated. The previous string is copied to a new location with the newly added part. This is inefficient. On the other hand we have StringBuilder which keeps the same position in the memory without performing the copy operation. Thanks to the strings’ appending by means of StringBuilder the process is much more efficient, especially in case of hundreds of append operations.

//INCORRECT
List values = new List(){"This ","is ","Sparta ","!"};
string outputValue = string.Empty;
foreach (var value in values)
{
   outputValue += value;
}

//CORRECT
StringBuilder outputValueBuilder = new StringBuilder();
foreach (var value in values)
{
   outputValueBuilder.Append(value);
}

2. LINQ – ‘Where’ with ‘First’ instead of FirstOrDefault

A lot of programmers find a certain set of elements by means of ‘Where’ and then return the first occurrence. This is inappropriate, because the ‘First’ method can be also applied with the ‘Where’ condition. What’s more, it shouldn’t be taken for granted that the value will always be found. If “First” is used when no value is found, an exception will be thrown. Thus, it’s better to use FirstOrDefault instead. When using FirstOrDefault, if no value has been found, the default value for this type will be returned and no exception will be thrown.

//INCORRECT
List numbers = new List(){1,4,5,9,11,15,20,21,25,34,55};
return numbers.Where(x => Fibonacci.IsInFibonacciSequence(x)).First();

//PARTLY CORRECT
return numbers.First(x => Fibonacci.IsInFibonacciSequence(x));

//CORRECT
return numbers.FirstOrDefault(x => Fibonacci.IsInFibonacciSequence(x));

3. Casting by means of ‘(T)’ instead of ‘as (T)’ when possibly not castable

It’s common that software developers use simple ‘(T)’ casting, instead of ‘as (T)’. And usually it doesn’t have any negative influence because casted objects are always castable. Yet, if there is even a very slight probability that an object can be under some circumstances not castable, „as (T)” casting should be used. See Difference Between C# Cast Syntax and the AS Operators for more details.

//INCORRECT
var woman = (Woman)person;

//CORRECT
var woman = person as Woman;

4. Not using mapping for rewriting properties

There are a lot of ready and very powerful C# mappers (e.g. AutoMapper). If a few lines of code are simply connected with rewriting properties, it’s definitely a place for a mapper. Even if some properties aren’t directly copied but some additional logic is performed, using a mapper is still a good choice (mappers enable defining the rules of rewriting properties to a big extend).

5. Incorrect exceptions re-throwing

C# programmers usually forget that when they throw an exception using „throw ex” they loose the stack trace. It is then considerably harder to debug an application and to achieve appropriate log messages. When simply using „throw” no data is lost and the whole exception together with the stack trace can be easily retrieved.

//INCORRECT
try
{
   //some code that can throw exception [...]
}
catch (Exception ex)
{
   //some exception logic [...]
   throw ex;
}

//CORRECT
try
{
   //some code that can throw exception [...]
}
catch (Exception ex)
{
   //some exception logic [...]
   throw;
}

6. Not using ‘using’ for objects disposal

Many C# software developers don’t even know that ‘using’ keyword is not only used as a directive for adding namespaces, but also for disposing objects. If you know that a certain object should be disposed after performing some operations, always use the ‘using’ statement to make sure that the object will actually be disposed.

//the below code:
using(SomeDisposableClass someDisposableObject = new SomeDisposableClass())
{
   someDisposableObject.DoTheJob();
}
//does the same as:
SomeDisposableClass someDisposableObject = new SomeDisposableClass();
try
{
   someDisposableObject.DoTheJob();
}
finally
{
   someDisposableObject.Dispose();
}

7. Using ‘foreach’ instead of ‘for’ for anything else than collections

Remember that if you want to iterate through anything that is not a collection (so through e.g. an array), using the ‘for’ loop is much more efficient than using the ‘foreach’ loop. See Foreach vs For Performance for more details.

8. Retrieving or saving data to DB in more than 1 call

This is a very common mistake, especially among junior developers and especially when using ORMs like Entity Framework or NHibernate. Every DB call consumes some amount of time and therefore it’s crucial to decrease the amount of DB calls as much as possible. There are many ways to do so:

• Using fetching (Eager Loading)
• Enclosing DB operations in transactions
• In case of a really complex logic, just moving it to the DB by building a stored procedure

It goes without saying that there are hundreds of other types of mistakes made by C# programmers. If you know any interesting ones or you want to share your opinion about the subject, feel free to leave a comment below! 

If you would like to read more about common mistakes of C# developers read this post.