Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

Analyzing Ignite.NET Code With NDepend

DZone's Guide to

Analyzing Ignite.NET Code With NDepend

See how NDepend works and how you can use it for code analysis to make sure your Ignite.NET code is clean and providing the best performance possible.

· Performance Zone ·
Free Resource

Maintain Application Performance with real-time monitoring and instrumentation for any application. Learn More!

Along with unit testing, continuous integration, and code review, static code analysis is invaluable for maintaining a healthy code base.

Ignite + NDepend

Ignite.NET uses FxCop and ReSharper code analysis from th early days, and this process is included in continuous integration on Ignite TeamCity, so I was fairly confident that our code is (mostly) fine.

Not so long ago, Patrick Smacchia reached out to me and suggested trying NDepend as well. Let’s see how it works out!

Getting Started With NDepend

We are going to work only with the Apache.Ignite.Core project: it is the biggest, most important, and most complicated part of Ignite.NET.

NDepend, like FxCop, operates on a built assembly (dll file), so we have to build the project in Visual Studio, open up the assembly in VisualNDepend.exe, and hit F5 (Run Analysis). The process is surprisingly quick: 1 second on my machine, where ReSharper (command-line) and FxCop take several seconds.

Upon analysis completion, we are presented with some statistics and a summary of issues:

NDepend Dashboard

2,654 issues! Whoops! No blockers, at least. But every static analyzer produces false positives and irrelevant issues, so it’s not time to worry yet.

Exploring Code Issues

There are 4 critical issues and 7 critical rule violations, we are going to start with these. Click on the red "4" to reveal a list of issues:

NDepend Critical Issues

Avoid Types Initialization Cycles (3 Issues)

Hovering over the “Avoid types initialization cycles” rule name shows a pop-up window with a detailed rule description along with possible false positive scenarios.

Our case seems like a false positive, since BinaryObjectBuilder does not have a static constructor and does not use BinarySystemHandlers in static field initializers.

The only usage of BinarySystemHandlers is on Line 127, but this has nothing to do with type initialization.

However, looking closer at how these three classes (BinaryUtilsBinaryObjectBuilderBinarySystemHandlers) relate reveals a real code quality issue: the method GetTypeId does not really belong in BinarySystemHandlers; it operates only on some constants from BinaryUtils. So, we could move GetTypeId to BinaryUtils, but that class is already ugly (“utility class anti-pattern”). The proper solution is to extract type code handling logic to a separate class: IGNITE-6233. So, even a false positive turned out to be quite useful.

Don’t Create Threads Explicitly (1 Issue)

This issue is simpler; new Thread(Run).Start() should be replaced with a ThreadPool or TaskIGNITE-6231. Exception handling is also missing in that thread, which can potentially bring down the entire process.

Moving on to High issues.

Avoid Namespaces Mutually Dependent (89 Issues)

The most violated High rule. It tells us that low-level namespaces (such as Apache.Ignite.Core.Impl.Binary) should not use higher-level namespaces (Apache.Ignite.Core). Following this rule provides a proper layered architecture where each namespace is kind of a “thing in itself” which can be moved anywhere, extracted to an external assembly, etc. This makes refactoring easier.

Such decoupling can be achieved by introducing additional interfaces. In the real world, however, introducing an interface just for the sake of decoupling within a single assembly is not feasible. Most of the core Ignite functionality is quite tightly coupled and is not expected to exist separately.

A good example of this is the serialization engine: it is inseparable from Ignite because it exchanges type metadata with other nodes separately.

This rule is still useful for baseline comparisons.

P/Invokes Should Be Static and Not Be Publicly Visible (21 Issues)

The IgniteJniNativeMethods class is internal static, so this looks like a false positive to me.

A Field Must Not Be Assigned From Outside Its Parent Hierarchy Types (17 Issues)

This is a very good rule, and Ignite.NET coding guidelines go even further by disallowing non-public fields entirely.

There are a couple of exceptions: UnmanagedCallbackHandlers is used solely for unmanaged interop; BinaryReader.Frame and BinaryWriter.Frame are private structs and are performance-sensitive.

Override Equals and Operator Equals on Value Types (15 Issues), Structures Should Be Immutable (7 Issues)

Again, very good rules. All public structs must follow it. Found issues are related to private structs, which have clearly defined use cases and are not used in comparisons and data structures.

Base Class Should Not Use Derivatives (3 Issues)

The open/closed principle in action. Two public classes violate this rule: TcpDiscoveryIpFinderBase and EvictionPolicyBase. These classes are explicitly closed for modification from outside of the assembly; constructors are internal. Only predefined derivatives are supported by the API.

Property Getters Should Be Immutable (2 Issues)

Relates to Choosing Between Properties and Methods MSDN guidelines. The main exception here is lazy initialization (TransactionsImpl.Current). The other issue is ClusterGroupImpl.TopologyVersion, which performs the expensive RefreshNodes() nodes operation and should really be a method: IGNITE-6370.

Conclusion

We have explored a small part of the issues found by NDepend, and already filed 3 tickets. The rule set is incredibly rich and covers many guidelines and best practices. I can recommend the tool for projects of any size.

As for Ignite.NET, we have decided to integrate NDepend with our CI pipeline: there is excellent TeamCity support.

Collect, analyze, and visualize performance data from mobile to mainframe with AutoPilot APM. Learn More!

Topics:
apache ignite ,ndepend ,.net ,ignite.net ,in-memory computing ,in-memory database ,performance

Published at DZone with permission of

Opinions expressed by DZone contributors are their own.

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}