Tuesday, January 12, 2010

A common mistake with FileInfo

This is an interesting one because it is usually not caught by mechanical tools. Consider the following three "equivalent" statements.

  • var x0 = new FileInfo(somefolder + filename);
  • var x1 = new FileInfo(String.Format("{0}{1}",somefolder , filename));
  • var x2 = new FileInfo(Path.Combine(somefolder, filename));
x0 will usually be caught by mechanical code and then recommend variation x1. Both x0 and x1 are problematic because they are very sensitive to the presence or omission of an ending "\" in somefolder.

The correct way is to use Path.Combine which both accommodate the "\" issue but does additional checking of the content.

Detecting redudant initializers

The code below illustrates two initializers, one of which is unneeded.

class StudentRecord
public StudentRecord(string studentNo)
StudentNo = studentNo;
public StudentRecord(string studentNo, string fullName)
StudentNo = studentNo;
FullName = fullName;
public string FullName { get; set; }
public string StudentNo { get; private set; }

Which could be initialize in two different manners:

  • var y = new StudentRecord("1234","John Doe" );
  • var x = new StudentRecord("1234") { FullName = "John Doe" };
The latter one is clearer, the former one calls for writing additional code that does contribute anything (except increase the risk of bugs and cost of maintenance).

Custom Initializers should be used when one of the following is true only:
  • Passes in data that that is not exposed as a property
  • Passes in data that is a property that is a private set.
To restate is in a simpler way:

var x=new StudentRecord( a,b,c,d);
can be rewritten as:
var x=new StudentRecord( a){paramb=b, paramc=c,paramd=d};
but cannot be rewritten as
var x=new StudentRecord(){ parama= a, paramb=b, paramc=c,paramd=d};

  • StudentRecord(object a) is appropriate
  • StudentRecord(object a, object b, object c, object d) is inappropriate, being redundant.

Monday, January 11, 2010

How to increase Lines of Code counts by bad initialation

I've seen old style coding which often results in HIGH lines of code counts. For example this code section could have been done in just ONE statement....

_operationsPerSecond = new PerformanceCounter();
_operationsPerSecond.CategoryName = _categoryName;
_operationsPerSecond.CounterName = "# operations / sec";
_operationsPerSecond.MachineName = ".";
_operationsPerSecond.ReadOnly = false;
_operationsPerSecond.RawValue = 0;

Instead of:

_TotalOperations = new PerformanceCounter(){CategoryName = categoryName,CounterName = "# operations executed",MachineName = ".",ReadOnly = false,RawValue = 0}

While in reality (looking at the defaults for the class), only the following is needed

_TotalOperations = new PerformanceCounter(){CategoryName = categoryName,CounterName = "# operations executed",ReadOnly =false};

I noticed that the original author had this initialator was already in the code base, so:

_TotalOperations = new PerformanceCounter(categoryName, "# operations executed", false);

OR, not using it.

_TotalOperations = new PerformanceCounter(){CategoryName = categoryName,CounterName = "# operations executed",ReadOnly =false}

This causes me to reflect on multiple initiators. Prior to (){ } implementation, they made sense. They no longer make sense -- unless the parameters are internal only OR have private set.

In this case, they are all read and write, so the best practise would be:

_TotalOperations = new PerformanceCounter(){CategoryName = categoryName,CounterName = "# operations executed",ReadOnly =false}

I will deal with the redundant initialators initializers in my next post.

Another factor that concerns me is the use of strings instead of an enumeration for CounterName. The counter name is not coming from user input, so using an adhoc string is very questionable.

The Starting point for this blog..

This classic book is still the best out there, although it is stale, being a reflection of 2004 state of best practices.

Practical Guidelines and Best Practices for Microsoft Visual Basic and Visual C# Developers (Pro-Developer) (Microsoft Press) Francesco Balena (Author), Giuseppe Dimauro (Author)

Free tools such as are good artifacts but also suffer from being stale:

Commercial tools such as:

Are often excellent for detecting a lot of coding issues, especially with recommended newer styles. The coverage of issues is likely 85-90%. There's are a variety of items that they do no catch.

This blog looks at issues that the above may not detect.