Code Style Quality

As a developer you should breathe code quality. Specially if you work with a team and it is important that others can understand faster what you wrote. But let’s see some advantages and examples on how to implement a Quality Code Style in a team.

ADVANTAGES

There are many, many reasons for a good code styling, the main ones are:
– it is good for a better maintenance;
– it must improve code review
– it saves disk space (but not always)
– it improves performance

So the key is: having a good code styling and make sure everyone will use the same code styling.

Implementing it

The 2 ways we have to implement it is with tools and code review.

There are several tools to help everyone to use the good standards of code styling, and some of them can integrate with build systems like Jenkins or TeamCity.

We can also define a threshold. After that threshold, build system should reject the build.

Stylecop

One of this styling extensions is Stylecop. With Stylecop we can avoid double empty space, code after a closing bracket or bad ordering of functions or members.

It is very easy to setup in development environments, as there is a Visual Studio extension and, integrating with TeamCity the build can fail if the stylecop detect a certain amount of violations (configured).

So, in the beginning, in order to implement the policy in a team, we need to make sure all the team understands the importance of code styling and start with one project, so the team can get used to.
Add the extension to that project and make an high threshold (like 50 violations).

After some time, we should reduce the threshold, until it reaches zero. Also we should progressively implement the same policy to all projects.

Peer Review

There is still the idea that peer reviewing is blaming someone. It is not. Peer Review is team building.
As a team we should be concerned with code quality and as a developer, because I am focused on a task, it is normal to make mistakes that others can catch.

Style coding should be included in peer reviews.
What should a reviewer highlight?

1. Dead spaces.

Dead spaces are spending bytes on our disk and for nothing.

It is true that space is not too expensive however if we have 100 files with dead spaces, let’s say an average of 50, it will be 50 * 100 bytes. And we all know that we have more than 100 files with 50 dead space each 🙂

There are some extensions for Visual Studio that help us to keep our code without any dead space:

a) Trailing Whitespace Visualizer

b) Trailing Space Flagger

2. Long lines.

This one will improve our team performance, as it is much easier to review a code with correct line breaks.

Specially if the reviewer uses a side by side code review layout. The recommended line lenght is 100 cols however 105 is acceptable.

There are also a extension to help on this. Actually is a package of extensions that most of the people is using called “Productivity Power Tools”

3. Wet code.

Wet code means “Not DRY code” where DRY means Don’t Repeat Yourself. It is a best practice and you can find more info on Wikipedia.

4. Too deeply code block.

A good example is when we have a chain of ifs without else and even if we can’t made a single if, we probably can invert the if most of the times.

(very very simple) Example:

public void function(){
    someCodeHere();
    if(someBooleanExpression){
        moreCodeHere();
        if(anotherBool){
            AndMoreCodeHere();
        }
    }
}

we can change this by:

public void function(){
    someCodeHere();
    if(!someBooleanExpression)
        return;

    moreCodeHere();
    if(!anotherBool)
        return;

    AndMoreCodeHere();
}
5. Chains of functions should be avoided

It is very common we try to reduce our code to only one line of code, but sometimes it is truly useful if we can see better what’s going on.

Example:

public string OneLineCode(bool isThisTrue){
    return isThisTrue
        ? someFunction(AnotherFunction(someGlobalVariable, someEnum.Where(x => x.SomeBoolExpression).Select(someSelectDelegation).FirstOrDefault()))
        : someFunction(thirdFunction(string.Empty, someOtherGlobalVar));
}

As we can see, this is really hard to understand. However, if we change it to

public string MoreThanOneLineCode(bool isThisTrue){

    if(isThisTrue){
        var parameter = someEnum.Where(x => x.SomeBoolExpression)
                            .Select(someSelectDelegation)
                            .FirstOrDefault());
        var someFunctionParameter = AnotherFunction(someGlobalVariable, parameter);
        return someFunction(someFunctionParameter);
    }

    var thirdFunctionReturn = thirdFunction(string.Empty, someOtherGlobalVar);
    return someFunction(thirdFunctionReturn);
}

We have 2 advantages: it is more readable and we can debug it in VS much better, as we have the values in vars for each step.

6. Unused variables.

It is very common when we refactor code, we left some unused variables forgotten in the code.

As a reviewer we should pay special attention to unused input variables and global variables as well.

One variant of this problem is unused Injected objects. This can also be caused by refactoring and it is very dangerous to be left if we don’t need it. Think in this example (It happened to me a couple of years ago).

I have an object than in the constructor has several injected objects and each one of this has injected objects etc.. This is a normal scenario and nothing abnormal is here.

The problem is when we don’t need to inject it in first instance.. We can spend some precious milliseconds (or even seconds) loading this chain for nothing. Also we are using unnecessary memory space. So this is a big problem to avoid.
(Tip. For performance debugging, this can be a possible bottleneck. Consider to lazy load your injected objects)

7. Duplicate empty lines.

Please, get rid of it.. Spaces in code are really awesome for readability however more than one empty line together is wast of space (\r\n are 2 chars, so 2 bytes (ASCII based) per empty line)

And there are many many more examples.