Skip to main content

Use Discretion when Reviewing Code Metrics

Code metrics are interesting creatures. Some are just raw numbers, such as depth of inheritance or lines of code, while others are a bit more subjective, like a maintainability index. But ultimately they are all meaningless without broader context and an understanding of the code.

As a brief example, consider the following C# function which accepts a string and returns the 40-character hexadecimal representation of the string's SHA1 hash.

public static String Sha1(String text)
{
using (SHA1Managed sha1 = new SHA1Managed())
{
Byte[] textBytes = Encoding.Unicode.GetBytes(text);
Byte[] hashBytes = sha1.ComputeHash(textBytes);
return Convert.ToBase64String(hashBytes);
}
}
The function accomplishes one task. Variables are defined closest to their usage. Function calls are clear and do not nest other function calls. Even using ( ) is used so the run-time can automatically dispose of the SHA1Managed resource. Yet a scan using Visual Studio 2010's Code Metrics returns a maintainability index of 71.

The index increases to 73 when the definition of variables move outside of the using ( ) block.
public static String Sha1(String text)
{
Byte[] textBytes, hashBytes;
using (SHA1Managed sha1 = new SHA1Managed())
{
textBytes = Encoding.Unicode.GetBytes(text);
hashBytes = sha1.ComputeHash(textBytes);
}
return Convert.ToBase64String(hashBytes);
}
Eliminating the variables altogether will increase the maintainability index to 76.

public static String Sha1(String text)
{
using (SHA1Managed sha1 = new SHA1Managed())
{
return
Convert.ToBase64String(sha1.ComputeHash(
Encoding.Unicode.GetBytes(text)));
}
}
76 is better than 71, but is the latter code really more readable and maintainable?

I posted this example to StackOverflow and asked if anyone knew why the metric would even increase in the first place. The responders agreed it is counter-intuitive, and the consensus is that it is better to focus on writing clean, concise, readable code.

An extremely low-scoring metric can be an indicator that something should be flagged for review, but use your discretion and judgment when reviewing the code. Use the report as a tool to identify possible problems and not a set of requirements to be met.

Comments

Popular posts from this blog

Geolocation Search

Services that allow users to identify nearby points of interest continue to grow in popularity. I'm sure we're all familiar with social websites that let you search for the profiles of people near a postal code, or mobile applications that use geolocation to identify Thai restaurants within walking distance. It's surprisingly simple to implement such functionality, and in this post I will discuss how to do so.

The first step is to obtain the latitude and longitude coordinates of any locations you want to make searchable. In the restaurant scenario, you'd want the latitude and longitude of each eatery. In the social website scenario, you'd want to obtain a list of postal codes with their centroid latitude and longitude.

In general, postal code-based geolocation is a bad idea; their boundaries rarely form simple polygons, the area they cover vary in size, and are subject to change based on the whims of the postal service. But many times we find ourselves stuck on a c…

Composing Music with PHP

I’m not an expert on probability theory, artificial intelligence, and machine learning. And even my Music 201 class from years ago has been long forgotten. But if you’ll indulge me for the next 10 minutes, I think you’ll find that even just a little knowledge can yield impressive results if creatively woven together. I’d like to share with you how to teach PHP to compose music. Here’s an example: You’re looking at a melody generated by PHP. It’s not the most memorable, but it’s not unpleasant either. And surprisingly, the code to generate such sequences is rather brief. So what’s going on? The script calculates a probability map of melodic intervals and applies a Markov process to generate a new sequence. In friendlier terms, musical data is analyzed by a script to learn which intervals make up pleasing melodies. It then creates a new composition by selecting pitches based on the possibilities it’s observed. . Standing on ShouldersComposition doesn’t happen in a vacuum. Bach was f…

Creepy JavaScript Tracking

I recently began allergy shots so my new Monday morning routine includes me sitting in a doctor's office for 30 minutes (I must wait after receiving the shots and be checked by a nurse to make sure there was no reaction). With nothing else better to do while I waited last week, I started playing around with some JavaScript. This is what I came up with:
<html> <head> <title>Test</title> <script type="text/javascript"> window.onload = function () { var mX = 0,  mY = 0, sX = 0,  sY = 0, queue = [], interval = 200, recIntv = null, playIntv = null, b = document.body, de = document.documentElement, cursor = document.getElementById("cursor"), record = document.getElementById("record"), play = document.getElementById("play"); window.onmousemove = function (e) { e = e || window.event; if (e.pageX || e.pageY) { …