Open-Source Code Quality – Foundatio

How important is code quality to your team? It should be at the top of the list, not only to make your customers happy but make your team happier when bugs arise and when features need to be added. Putting quality in your code in the future is a lot more expensive than doing it when the code is first written. How does your open-source solution compare to some of the most popular GitHub/ NuGet packages? Do you analyze repositories before using them or just adding them via NuGet? If not, you need to keep reading.

Foundatio

Code Quality Rating

https://github.com/FoundatioFx/Foundatio

15.42

Foundatio is a pluggable foundation blocks for building loosely coupled distributed apps which includes implementations in Redis, Azure, AWS, RabbitMQ, and in memory (for development). I have not used these projects, but it looks very interesting. The code quality rating shows there is a code violation every 15.42 lines of code, which is the best rating I have seen since starting these articles.

Errors

Warnings

Information

Unit Test Coverage

919

1,232

3,520

249 – Broken

Maintainability Index

Cyclomatic Complexity

Depth of Inheritance

Class Coupling

89

20,675

673

18,538

Lines of Source Code

Lines of Executable Code

Lines of Cloned Code

Code Commenting

87,429

37,212

717

Grade of D

Coding Standard Issues

Here are just some examples of what needs to be fixed from the 5,671 violations in this solution.

Using ConfigureAwait

All the 919 errors above were this issue: CA2007 – Consider calling ConfigureAwait on the awaited task. Here is an example of the code that violates this rule:

await storage.SaveFileAsync(path, memoryStream);

This is fixed by adding ConfigureAwait() as recommended by Microsoft for DLL’s:

await storage.SaveFileAsync(path, memoryStream).ConfigureAwait(continueOnCapturedContext: false);

Braces Around If Statements

One of the issues I see a lot is not properly adding braces to if statements. It is my feeling that this is due to software engineers coming from other languages like Java. Here is an example:

if (_logger.IsEnabled(LogLevel.Trace)) _logger.LogTrace("Setting Local cache key: {Key} with expiration: {Expiration}", key, expiration);

Here is how to fix this violation:

if (this._logger.IsEnabled(LogLevel.Trace))
{
    this._logger.LogTrace("Setting Local cache key: {Key} with expiration: {Expiration}", key, expiration);
}

This library also does not use proper braces placement for else, foreach, lock, using, and while statements. There is a lot of cleanup work that should be done to fix these violations.

Accessibility Modifiers

Something very important for object-oriented programming is to property set up classes for their intended use. Every class, property, and method should have a specific accessibility modifier specified. This is very important. Here is an example of the violation:

class RegisteringMemoryStream : MemoryStream, IDisposable

It should have been defined like this:

private class RegisteringMemoryStream : MemoryStream, IDisposable

Here is an example of an issue with a field:

readonly IMessageSink diagnosticMessageSink;

It should have been defined like this:

private readonly IMessageSink diagnosticMessageSink;

Pattern Matching

Pattern matching is a way to speeding up your code. Here is an example of the violation:

if (!(metrics is IMetricsClientStats stats))
    return;

This violation can be fixed like this:

if (metrics is not IMetricsClientStats stats)
{
    return;
}

Inlining Out Variables

Something newer to .NET is the way we create variables for out parameters. The older way of doing this is shown below:

object value;
if (_structAsObjectCache.TryGetValue(type, out value)) return value;
value = adder(type);

Here is how to fix this violation:

if (_structAsObjectCache.TryGetValue(type, out var value))
{
    return value;
}

I have never been a big fan of out parameters, but at least doing the variables inline makes the code a bit cleaner.

Other Common Violations

Coding Standards Book Cover-E7@0.5xHere are some other common issues I see in this code base. I have previously written about all of these issues either in my coding standards book or online.

  1. Multiple classes in the same file.
  2. Unnecessary assignment of values.
  3. Classes are not organized as per StyleCop rules.
  4. Missing file header documentation.
  5. Much of the code is missing (XML) documentation.
  6. The implementation of IDisposable is not implemented properly in classes.
  7. Fields that should be read-only are not properly defined.
  8. I could not find any methods that properly validated the parameters. This is so important for proper object-oriented programming. This means everything I analyzed breaks encapsulation. The codebase has only 249 unit tests. That number needs to be over 2,500 just to properly test encapsulation!!

Summary

Now that I have analyzed these projects, I think I will wait until they fix all the errors and warnings and the IDisposable issues. Therefore, it is so important to analyze projects before just adding them to your projects.

To learn the proper way of implementing coding standards for .NET, please pick up a copy of my coding standards book on Amazon http://bit.ly/RockYourCodeBooks!

Go here to view the .editorConfig that I use to analyze projects: https://gist.github.com/RealDotNetDave/dbae4d97358ba4515dd52e5b8ca87671

I hope that the next time you want to download a NuGet package for your solution or use an open-source library, you will analyze it first for code quality. I plan to analyze more libraries in the future as a blog post. What library would you like me to analyze? Please comment below.

Leave a Reply

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.