Anders Norås' blog

Replacing Constructors with Creation Methods is a bad idea

One of the patterns in Joshua Kerievsky’s “Refactoring to Patterns” is “Replace Constructors with Creation Methods”. The motivation behind this pattern is that programmers might be confused by classes with multiple constructors and that constructors do not communicate their intention very well. This is true; I have often found myself poking around in classes for the right constructor to use. Furthermore, overloaded methods in general have shortcomings since it is impossible to have two methods or constructors with the same signature.
The proposed solution to tackle the confusion caused by having multiple constructors on a class is to exchange the overloaded constructors with descriptive creation methods. When the creation methods are in place all calls to the different constructors should be exchanged with calls to the corresponding creation method. The different constructors should then be consolidated in one single private constructor used by all creation methods.
The example in the book uses a Loan class with multiple constructors and swaps these with static methods that create different kinds of loans such as term and revolver loans.

The listed benefits are:

• Creation methods communicate available instances better than constructors.
• Bypasses the signature limitations.
• Makes it easier to discover unused constructors.

Nonstandard object creation is the only liability recognized in the book. There are additional important liabilities. The refactoring changes the public interface. All client code must be changed to use the creation methods instead of the new keyword.
The class is rendered unserializable after the refactoring since classes must have a public constructor in order to be serializable.

There is a much better solution to the issues this pattern addresses. All developers are familiar with it; inheritance.
Instead of re-implementing the “is-a” relationship of inheritance with methods, one could create different Loan classes that would describe the different types of loans much better than the creational methods do.
For instance one could simply create a RevolverLoan class that extends the Loan class. This class could have fewer constructors that make sense in the business context a revolver loan is used.
The signature limitations would be addressed as well since a subclass would have fewer and more precise constructors eliminating the need for different constructors with the same signature.
The argument that unused constructors are discovered more easily is based on that every object creation has to be replaced. Unless you have complete control over the client code, this is not a benefit at all, it is a liability. Most modern development environments will automatically identify unused code, static analyzers will do the same and if you have proper unit testing in place, code coverage tools will do it as well.
If the original base class is kept as is, the existing client code does not have to be changed since it can keep on creating instances of the original class as usual. New client code can start using the different subclasses and client code can be refactored to use the specific subclasses over time. When the base class no longer is created directly it can be made abstract if this is appropriate.

Refactoring to patterns is a great idea, but one has to be careful so one does not refactor to anti-patterns.

The inner works of code coverage instrumentation in VS Team System

A Norwegian bank account number is 11 digits long. This means that there are 10¹¹ possible values for such a number. If you were to test an account number validation method you would not feed it all possible numbers since only few of those number are likely to be valid. Instead you pick a few numbers that should pass the test and some that should not. A banking domain specialist is able to pick these numbers because she knows how an account number is built. In Norwegian banks these consist of a four digit bank id, a two digit account type and a five digit account number. From the account number a checksum can be calculated to validate whether the number is a possible account number or not. It is this knowledge you are building into your validation method.
When implementing such a validation method you're likely to use quite a few if statements or case switches and maybe a couple of for loops as well. Each of these instructions gives your method an additional execution path for which a test case should be written.
Structured basis testing is a technique for testing all possible paths through a method. The basic concept in structured basis testing is that you need to test each statement at least once. If the statement is a conditional statement you need to vary the test cases to test all possible outcomes to ensure the statement is fully tested.

In the 1976 Thomas McGabe pioneered a metric for cyclomatic complexity. Cyclomatic complexity is in essence the number of execution paths for a section of code.
The formal equation for the metric is CC=E-N+P where E is the number of edges in a graph, N the number of nodes and P the number of connected components.
Nodes are decision statements such as if and edges are paths between nodes. In object-oriented languages P has a constant value of 1 representing the method entry point.
You can calculate the number of test cases needed to cover the entire method and code coverage tools help you determine which parts of your code your test cases cover and which parts they don't.

1private accountType GetAccountType

2{

3 get

4 {

5  if (Balance > 15.00)

6   return accountType.Platinum;

7  else

8   return accountType.Gold;

9 }

10}

11

Consider the above method from the Woodgrove Bank walkthrough project.  This method has one decision point; the if statement. Adding the constant 1 for the methods entry point yields a cyclomatic complexity of 2. From this we can determine that at least two unit tests are required to thoroughly test the method.
When running the test from my previous post on private method testing, the code coverage results show that 25% of the GetAccountType property is not covered by the test case. When the "Show code coverage" switch is on, Visual Studio even shows you which lines are covered (the green ones) and which aren't (the red ones). How does Visual Studio 2005 Team System know this?
To determine which parts of a method a test covers code coverage tools instrument the test target. The Visual Studio 2005 Team System code coverage analyzer injects probes into the executable to monitor which lines the execution pointer stops by and which are never visited.

1private BankAccount.accountType get_GetAccountType()

2{

3      BankAccount.accountType type1;

4      Init_0e94efa84444b4ee4b7ec08941916a9b.Register();

5      Init_0e94efa84444b4ee4b7ec08941916a9b.m_vscov[9][14] = 1;

6      Init_0e94efa84444b4ee4b7ec08941916a9b.m_vscov[9][15] = 1;

7      bool flag1 = this.Balance <= 15;

8      if (!flag1)

9      {

10            Init_0e94efa84444b4ee4b7ec08941916a9b.m_vscov[9][0x10] = 1;

11            type1 = BankAccount.accountType.Platinum;

12      }

13      else

14      {

15            Init_0e94efa84444b4ee4b7ec08941916a9b.m_vscov[9][0x11] = 1;

16            type1 = BankAccount.accountType.Gold;

17      }

18      Init_0e94efa84444b4ee4b7ec08941916a9b.m_vscov[9][0x12] = 1;

19      return type1;

20}

The above code is the GetAccountType property from earlier with the probes inserted. Init_0e94efa84444b4ee4b7ec08941916a9b is a generated class. This class is accompanied by a structure named Header_0e94efa84444b4ee4b7ec08941916a9b. As you might have guessed, the awkward name ensures that the class and structure names are unique. The Init class has a static internal UInit32 array named m_vscov.
The first call registers the method for instrumentation. This is done by passing a reference to the m_vscov array to a method called VSCoverRegisterAssembly on an unmanaged dynamic link library named vscover.dll.
The subsequent injected lines are the actual probes. The m_vscov map that was passed to the external method is used to keep track of execution. The first dimension of the array seems to always have 9 as its index. The second dimension corresponds with an address in an instance of the header structure (m_RvaVscov).
Each time a probe is passed by the execution pointer, a flag is set to 1 on the probes unique index. The number 1 is a numeric representation of a true boolean.
When a test run completes the Visual Studio code coverage analyzer can retrieve the code coverage metrics by analyzing the flags in the m_vscov map.
The number of lines not covered is the total number of probes minus probes with the flag set to true and so on.
The code coverage probes do not end up in neither debug nor production code. Instead Visual Studio compiles a separate executable used solely for code coverage instrumentation. This file will be located in Documents and Settings\%user%\Local Settings\Application Data\VSEqtDeploymentRoot\%guid%\Out\.
Since the actual executable is modified, Visual Studio also provides an option for re-signing the assembly after a code coverage metric have been collected. This option is found on the "Code Coverage" tab in the "Select a Test Run Configuration"dialog.

To make sure the entire method is covered by the test we have to extend the test case to include a test where the balance is over the minimum for the platinum account; $15.00.

1[TestMethod()]

2public void GetAccountTypeTest()

3{

4 string customerName = null;

5 double balance = 0;

6 BankAccountNS.BankAccount target = new BankAccountNS.BankAccount(customerName, balance);

7 BankAccountNS.BankAccount.accountType val = BankAccountNS.BankAccount.accountType.Gold;

8 BankAccount.BankAccountAccessor accessor = new BankAccount.BankAccountAccessor(target);

9 Assert.AreEqual(val, accessor.GetAccountType);

10 balance = 16.00;

11 target = new BankAccountNS.BankAccount(customerName, balance);

12 val = BankAccountNS.BankAccount.accountType.Platinum;

13 accessor = new BankAccount.BankAccountAccessor(target);

14 Assert.AreEqual(val, accessor.GetAccountType);

15}

Volia, all probes get hit and we have 100% code coverage.

This is why private method testing is good

My coworker Mads Nissen has posted a comment to my post about Visual Studio Team System’s support for private method testing. He discusses private method testing from a test driven development point of view and argues that private method testing has no place in this methodology. He even goes to the extent of stating that private method testing only will be used by inexperienced programmers. I disagree with him on this.

When doing contract-first development and applying test driven methodologies private method testing is not applicable since neither the contract nor the test case describe the inner working of the test target. The built in support in Team System for private method testing wouldn’t even be available in such a scenario since it generates the tests and the proxies to support them based on existing code. When tests are written prior to the target code there is no code to generate tests and proxies from.

The IEEE Standard Computer Dictionary defines unit testing as “Testing of individual hardware or software units or groups of related units”. Since methods within a class rely on other methods within the class or other classes, the interface testing Mads refers to is, from my point of view, testing groups of related units. A unit test usually exercises some particular method in a particular context. Interface or “black box” testing exercises the target in a context where internal methods are interrelated.
You design public methods to be “black boxes” so that the users of a class don’t have to look past the interface to know what the methods do. Having “black box” tests that verify that the public interface functions as intended is a necessity, but in addition you should have “glass box” tests that exercise the individual units within the “black box”. Having knowledge about what is behind the public interface enables you to test the code more thoroughly and gives you a better chance of detecting errors.
If you have a private method used from a public method you should create individual tests for this method. The reason is simple. When basic, low-level code isn’t reliable, the requisite fixes don’t stay at the low level. You fix the low level problem, but that impacts code at higher levels, which then need fixing, and so on. Fixes begin to ripple throughout the code, getting larger and more complicated as they go. Tests ensure that the code is reliable.
In addition testing individual methods reduces the problem scope and makes it easy to write good tests.
The “black box” tests should stick to a class for its entire life time to ensure that the class fulfills its intent. The “glass box” tests should be regarded as technical scaffolding and should be changed with the target code. If a refactoring renders a private method redundant the tests should be refactored to reflect this.

There are some interesting side-effects to testing. One of these is that tests are “executable documentation” of the target’s intended uses. Having a set of tests for the internals of a class make it easy for a maintains-programmer to understand what the code does. Therefore I also disagree that writing tests for private methods reduce the maintainability of the code.

There are many levels of testing and unit tests make up the lowest level. In addition integration tests, acceptance tests, value-chain test and so on must be part of software development projects. Unit tests should test the standalone units. A unit might be a public interface on a class when viewing in the class from the outside or the internals of the class when viewing the class from the inside. There is no conflict between testing the interface and testing the internals, so there is no reason not to do both. With NUnit and other testing tools testing private methods has been very cumbersome and therefore it has seldom been done. Having support for this in Visual Studio Team System is therefore a step in the right direction.

The inner works of private method testing in VS Team System

While researching for a presentation of code quality tools in Visual Studio .NET 2005 Team System I discovered that you can create unit tests for private methods in your test fixtures. Since the target and the test fixture are two separate classes they do not have access to each others private members, still the Team System unit testing framework enables you to access these. As a curious cat, I had to investigate how this is accomplished.
The following snippet is taken form the Woodgrove Bank walkthrough project's BankAccount class:
1// private method:
2private accountType GetAccountType
3{
4 get
5 {
6 if (Balance > 15.00)
7 return accountType.Platinum;
8 else
9 return accountType.Gold;
10 }
11}
And this is the unit-test code generated by the Team System unit test generator:
1/// <summary>
2/// GetAccountTypeTest is a test case for GetAccountType
3/// </summary>
4 [TestMethod()]
5 public void GetAccountTypeTest()
6 {
7 // TODO: Initialize to an appropriate value
8 string customerName = null;
9 // TODO: Initialize to an appropriate value
10 double balance = 0;
11
12 BankAccountNS.BankAccount target = new BankAccountNS.BankAccount(customerName, balance);
13
14 // TODO: Assign to an appropriate value for the property
15 BankAccountNS.BankAccount.accountType val = BankAccountNS.BankAccount.accountType.Gold;
16
17 BankAccount.BankAccountAccessor accessor = new BankAccount.BankAccountAccessor(target);
18 Assert.AreEqual(val, accessor.GetAccountType);
19
20 Assert.Inconclusive("Verify the correctness of this test method.");
21 }
Take note of the highlighted line. Instead of creating an instance of the BankAccount class directly the test case uses a BankAccountAccessor class. This class is also used to access fields and methods. However this class is not a part of the project or is it?
While no class is shown in the solution explorer, you will find a source file called CodeGenAccessors.cs in the project directory. This is an auto generated file and it contains the various accessor classes for your target.
The following snippet shows the proxy for the private GetAccountType method.

1    public BankAccountNS.BankAccount.accountType GetAccountType {
2 get { 
3    return ((BankAccountNS.BankAccount.accountType)(m_privateObject.GetProperty("GetAccountType"))); 
4     } 
5 } 
6

Still the generated accessor is still a separate class from the test target, so how does this work?
The answer is simple. The testing framework applies a little reflection magic beneath the covers. The m_privateObject field is an instance of Microsoft.VisualStudio.QualityTools.UnitTesting.Framework.PrivateObject which uses the Type’s InvokeMember method to invoke the get property in the above example.
To use this technique in your own code simply do this:
1public class Thief {
2 public static void Main()
3 {
4 Safe somebodyElsesSafe=new Safe();
5 Console.WriteLine((string)somebodyElsesSafe.GetType().InvokeMember("secret",BindingFlags.GetField|BindingFlags.Instance|
6BindingFlags.NonPublic,null,somebodyElsesSafe,null));
7 }
8}
9public class Safe {
10 private string secret="This is a secret lock inside a safe!";
11}

This is a cunning solution to a classic unit testing problem. And although extra code is included in your project while testing it is much more elegant than making private members public, which was the previous technique of choice, while testing.

Attention, Norwegian .NET bloggers!

At Microsoft’s partner kick-off in Oslo last Wednesday, I spoke with Petter Schatvet, who is the Norwegian developer tools product manager, about compiling a list Norwegian Microsoft centric blogs. The list is to be published at the Norwegian .NET User Group (NNUG) homepage and links to key blogs might also be included on the Norwegian MSDN site.
I know a few Norwegian bloggers, but there are probably many more out there. Therefore I call upon all Norwegian bloggers to submit a link to their blog either by commenting on this post or by mailing me.
Below is list of all Norwegian blogs on my roll.

Anders Norås	Yours truly!	http://andersnoras.info or http://dotnetjunkies.com/weblog/anoras
Andreas Brunvol	Microsoft Architecture Advisor	http://blogs.msdn.com/abrunvol
Andreas Eide	MVP, Regional Director and President of NNUG	http://weblogs.asp.net/andrease/
Einar Ingebritsen	Development manager In Notus Systems	http://blog.dolittle.com/
Janok	Information for Norwegian Microsoft Partner	http://weblogs.asp.net/janok/
Mads Nissen	An Objectware colleague	http://weblogs.asp.net/mnissen/
Morten Abrahamsen	Software Architect at Ibistic Technologies AS	http://blog.morty.info/
Roy Tore Gurskevik	Another Objectware colleague	http://dotnetjunkies.com/WebLog/rtgurskevik/

OptimizationCanBeRefactoring

Martin Fowler recently added a discussion on whether optimization is refactoring to his bliki. His conclusion is that optimizations are not refactorings, but that a particular change made to a program might be both.
I believe that unless you are working with inefficient programming environments such as browser scripting or you have extreme performance requirements you should avoid performance optimiz