ITSWITCH #1: Answer

Last post I detailed some code that may or may not have something wrong in it.  If you thought InitializeOne and IntializeTwo are semantically identical (e.g. they differ only by performance), you’d be wrong.

If you simply ran the code, you’d be able to guess where the problem is.  To understand what’s causing the problem.  Let’s look at how C# effectively implements the two loops.

InitializeOne is essentially equivalent to

        private class PrivateDelegateHelper
        {
            public String Value { get; set; }
            public void Method()
            {
                TestClass.ProcessText(Value);
            }
        }
 
        public void InitializeThree(String[] strings)
        {
            delegates = new List<MethodInvoker>(strings.Length);
            MethodInvoker cachedAnonymousDelegate = null;
            PrivateDelegateHelper privateDelegateHelper = new PrivateDelegateHelper();
            String[] copyOfStrings = strings;
            for(int i = 0; i < copyOfStrings.Length; ++i)
            {
                privateDelegateHelper.Value = copyOfStrings[i];
                if (cachedAnonymousDelegate == null)
                {
                    cachedAnonymousDelegate = new MethodInvoker(privateDelegateHelper.Method);
                }
                delegates.Add(cachedAnonymousDelegate);
            }
        }

Now it’s obvious, right?

For those you don’t want to read all the code, the problem is that only one PrivateDelegateHelper object is instantiated and its value property is set in each iteration of the loop.  Because the delegates aren’t run until sometime after the loop, they’re all run with the last value of the string array as their argument.

The technical term for what we’ve implemented here is a closure.  If you’re using Resharper 4.x, you would have noticed a warning "Access to modified closure":

 

…which is attempting to tell you that the closure (the delegate and cached bound variables) has changed (in this case one of the bound variables has changed between the creation of a closure and another and out expected output is effected).

By the way, you can get the same thing with C# 3+ with lambdas (i.e. you can also write closures with lambdas):

        public void InitializeOne(String[] strings)
        {
            delegates = new List<MethodInvoker>(strings.Length);
            for (int i = 0; i < strings.Length; ++i)
            {
                String value = strings[i];
                delegates.Add(() => ProcessText(value));
            }
        }
 
        public void InitializeTwo(String[] strings)
        {
            delegates = new List<MethodInvoker>(strings.Length);
            foreach(String value in strings)
            {
                delegates.Add(() => ProcessText(value));
            }
        }

ITSWITCH: #1

Is There Something Wrong In This Code Here

UPDATE: as several readers pointed out there was compile error in the code what was being displayed.  The line “String value = string[i]” was showing up as “String value = string”.  I’m not sure why; but throwing some spaces between the i and the square brackets seems to have solved it.

In the following class, is there something wrong with either InitializeOne or InitializeTwo (hint, it has nothing to do with compile errors and I’m ignoring performance differences)?

    public class TestClass
    {
        private List<MethodInvoker> delegates;
 
        private static void ProcessText(String text)
        {
            Trace.WriteLine(text);
        }
 
        public void Run()
        {
            foreach(MethodInvoker invoker in delegates)
            {
                invoker();
            }
        }
 
        public void InitializeOne(String[] strings)
        {
            delegates = new List<MethodInvoker>(strings.Length);
            for (int i = 0; i < strings.Length; ++i)
            {
                String value = strings[ i ];
                delegates.Add(delegate { ProcessText(value); });
            }
        }
 
        public void InitializeTwo(String[] strings)
        {
            delegates = new List<MethodInvoker>(strings.Length);
            foreach(String value in strings)
            {
                delegates.Add(delegate { ProcessText(value); });
            }
        }
    }

Answer on Monday.

Working with Resharper’s External Annotation XML Files

Resharper 4.0 has external annotation XML files that you can create to give Resharper more information about your code.  For example, you can tell Resharper that a particular method does not accept a null argument.  For example, the following method does not accept a null argument:

using System;
 
namespace Utility
{
    public static class Text
    {
        public static int GetLength(String text)
        {
            if (text == null) throw new ArgumentNullException("text");
            return text.Length;
        }
    }
}

An external annotation file can be created to inform Resharper of this fact and have it warn you when possible null values are passed as an argument:

            String text = null;
            Utility.Text.GetLength(text);

First, you need to create a directory within the Resharper ExternalAnnotations directory.  This ExternalAnnotations directory is usually in the form of “%SystemDrive%\Program Files\JetBrains\ReSharper\vBuild#\Bin\ExternalAnnotations” for example “C:\Program Files\JetBrains\ReSharper\v4.0.816.4\Bin\ExternalAnnotations”.

The directory we need to create is the same name as our assembly (without the extension).  In our case this would be “C:\Program Files\JetBrains\ReSharper\v4.0.816.4\Bin\ExternalAnnotations\Utility”.

Next, we need to create an XML file to contain the information required by Resharper.  The name of this file is the same as the directory name, plus “.xml”.  So, the full file name would be “C:\Program Files\JetBrains\ReSharper\v4.0.816.4\Bin\ExternalAnnotations\Utility\Utility.xml”.  This file essentially has an assembly element with child member elements that provide meta-data about methods and arguments.  In our case we want to tell Resharper that the Utility.Text.GetLength(String) method does not accept null values for the argument named “text”.  To do this we populate the file with the following XML:

<?xml version="1.0" encoding="utf-8" ?>
<assembly name="Utility">
    <member name="M:Utility.Text.GetLength(System.String)">
        <parameter name="text">
            <attribute ctor="M:JetBrains.Annotations.NotNullAttribute.#ctor" />
        </parameter>
    </member>
</assembly>

After restarting Visual Studio, Resharper will now warn that “text”, when passed to GetLength, in the following code, has a “Possible ‘null’ assignment to entity marked with “NotNull” attribute”.

            String text = null;
            Utility.Text.GetLength(text);

I’ve tried this with the GA build and with both Visual Studio 2005 (SP1) and Visual Studio 2008.


For more information about the annotation options, search for AssertionConditionType in the Resharper help and browse around “%SystemDrive%\Program Files\JetBrains\ReSharper\vBuild#\Bin\ExternalAnnotations” for examples.

Drag-copying in Visual Studio Solution Explorer.

NOTE: I’ve tried this in Visual Studio 2008 (VS2k8), I’m assuming the same thing happens in Visual Studio 2005 (VS2k5).


In the process of refactoring, it’s *very* common for me to rename a type.  This is most easily done by renaming the file in the Solution Explorer (SE)–which renames the file, the type, and and any uses of the type in the entire solution.


Occasionally, I need to create a new type based on another.  Copying an abstract type in order to implement the abstract type is often handy–I just fill in the abstract members (and delete “abstract”) in the copied type after renaming it.


Drag-copy in the SE seems like it would take care of a couple of steps at once for me: make a copy and rename it.  Unfortunately it doesn’t do that.  It makes a copy of the file (as “Copy of typename.xx”) but doesn’t rename any types in the class that match the file name.


This might seem somewhat trivial… I can simply rename the file name then refactor rename the type in the file so that the type and all it’s constructors are renamed in one fell swoop.  Alas, this simply opens a can of worms that can completely confound a newcomer and annoy an expert.


That simple, intuitive method of renaming a copy of a file then the type within the file actually renames *all* types of that name.  Since we’ve just made a copy of the type, that means it’s always going rename types in two files.  The side-effect of drag-copy in SE means you *must* manually rename the type in the file you just copied.  You can do this with search-replace; but that’s friction I don’t want and really makes SE drag-copy unusable.


I’ve logged a bug about it on Connect; but the olde “by design” card was played…

Multi-platform Testing

Sometimes in the testing of our code we need to ensure what we’ve written works in various environments.  Sometimes that’s pre-XP Windows, low-resolution computers, etc.


One way is to have a group of computers on hand with various versions of Windows and configurations installed for testing.  Another is to have a series of virtual PCs on your computer that can be run.  Sometimes you can use your running version of Windows…


There are compatiblity settings in XP and greater that allow you to tell Windows to ignore certain features when running an application.


Windows XP and greater as a concept of Visual Styles.  There lots of .NET APIs you can use that use Visual Styles, like ComboBoxRenderer.  Unforunately, if you run the code on a Pre-XP version of Windows or an XP or greater version of Windows with Visual Styles disabled, then you’ll get an InvalidOperationException.  Testing in XP with Visual Styles enabled won’t let you find these bugs.  But, you can test without Visual Styles without having to change your display settings or deploy to another computer (physical or virtual) and run.  In the compatibility settings for an application you can tell it to disable Visual Styles.


 


Compatibility is a very helpful little feature, it also lets you test what you application will lool link in minimum display situations like 640×480 and 256 colours without having to endure those settings during development.

Nested Types

Recently Michael Features blogged about nested types.  The title was almost “nested types considered harmful”.

I don’t agree.  I don’t agree that they’re any more harmful than any other C# construct (except goto…).  Nested types are like anything else in our tool-belt: they have a time and place and can be abused.

But, when to use them?  Well, for the most part I agree with Michael, you should avoid them. But, there are times when they’re simply the best solution in a given set of circumstances.

Let’s look at asynchronous programming model (APM) in .NET.

        // Paraphrased from MSDN
        // Accept one client connection asynchronously.
        public static void DoBeginAcceptTcpClient(TcpListener
            listener)
        {
            // Start to listen for connections from a client.
            Trace.WriteLine("Waiting for a connection...");
 
            // Accept the connection. 
            // BeginAcceptSocket() creates the accepted socket.
            listener.BeginAcceptTcpClient(
                DoAcceptTcpClientCallback,
                listener);
        }
 
        // Process the client connection.
        public static void DoAcceptTcpClientCallback(IAsyncResult ar)
        {
            // Get the listener that handles the client request.
            TcpListener listener = (TcpListener)ar.AsyncState;
 
            // End the operation and display the received data on 
            // the console.
            TcpClient client = listener.EndAcceptTcpClient(ar);
 
            // TODO: do something with client.
 
            // Process the connection here. (Add the client to a
            // server table, read data, etc.)
            Trace.WriteLine("Client connected completed");
        }
 

In this simple scenario we are getting by with a state of simply a TcpListener object.  In a more complex scenario, you’ll likely also want a connection-specific queue, some sort of information about what to do after a connection, etc.  While you can use existing types of have several collection instance fields to keep track of each of these things; you then have to introduce synchronization of those collections, managing the content of those collections, etc.–it’s much easier and safer to send that information on the stack.  One method I’ve tried is simply passing an Object collection as the state; but that quickly becomes hard to manage because of the lack of type-safety on the elements in the array (if I remove an element and replace it with another type, the compile can’t know and I’ll get a run-time error instead of a compile-time error).  To get type safety I generally introduce a new type to aggregate all the types I need in this asynchronous callback.  While this new type *could* be reusable by other classes; it likely isn’t and I don’t want to then be bound that that explicit contract I’ve signed by making the types publicly available.  The only option of not making them publicly available is as private nested types.  For example:


        private class AcceptTcpClientParameters
        {
            public CommandQueue CommandQueue { get; private set; }
            public Command NextCommand { get; private set; }
            public TcpListener TcpListener { get; private set; }
 
            public AcceptTcpClientParameters(int commandQueue, int nextCommand, TcpListener tcpListener)
            {
                CommandQueue = commandQueue;
                NextCommand = nextCommand;
                TcpListener = tcpListener;
            }
        }
 
        // Accept one client connection asynchronously.
        public static void DoBeginAcceptTcpClient(TcpListener
            listener, CommandQueue commandQueue, Command nextCommand)
        {
            // Start to listen for connections from a client.
            Trace.WriteLine("Waiting for a connection...");
 
            // Accept the connection. 
            // BeginAcceptSocket() creates the accepted socket.
            listener.BeginAcceptTcpClient(
                DoAcceptTcpClientCallback,
                new AcceptTcpClientParameters(commandQueue, nextCommand, listener));
        }
 
        // Process the client connection.
        public static void DoAcceptTcpClientCallback(IAsyncResult ar)
        {
            AcceptTcpClientParameters parameters = ar.AsyncState as AcceptTcpClientParameters;
            if(parameters == null) return;
 
            TcpClient client = parameters.TcpListener.EndAcceptTcpClient(ar);
 
            parameters.NextCommand.Process(parameters.CommandQueue, client);
        }

I find this use of nested types to be more object-oriented (the needs of the DoAcceptTcpClientCallback are abstracted), more intention revealing, better implements Single Responsibility Principle (SRP), better separates concerns, more maintainable and more agile.


Now, to be clear; this is forced set of circumstances.  You’re using a library that implements the APM (right?  You haven’t implemented APM yourself…).  But, that’s my point–nested types are almost essential in a given set of circumstances.


kick it on DotNetKicks.com