Showing posts with label C#. Show all posts
Showing posts with label C#. Show all posts

Wednesday, April 9, 2014

Are enumerations evil?

Or, How to refactor an enumeration to multiple classes

I ran across this article about how to refactor a switch statement to a data structure in JavaScript a while ago. This is not a terrible refactoring technique. I've used something similar with dictionaries in C# where the key was an enumeration and the value was an Action (or Func). It can be a minor refactoring to a single method that can help clean-up the code. Then this week I had a conversation with a fellow developer about the appropriateness of an enumeration in existing code from a couple different libraries he was reading.

This led me to wonder afresh if perhaps enumerated types aren't something that we as the development community should treat as an anti-pattern. In the days before class types, they were a means for the compiler to force integers to known values and provide more readable code for the developer. The compiler could check values better at compile time and catch usage errors earlier. I have worked on languages without them and embraced them when they became available. At that point they were a good thing.

Many times enumerations are used to change behavior of some code based on a variable's value. This is usually done through switch or if-then-else statements. Or, as discussed above, using some sort of dictionary like structure to store behavior associated with a specific enumerated value.

The problem is this pattern tends to get replicated within the class containing the enumerated variable. The class using the enumeration needs to do different things based on the various values the variable can have and these different things spread throughout the class (or classes) using the enumeration. Then, when a value is added to the enumeration, each place the variable is tested needs to be updated to handle the new value.

This can lead to a number of problems. With behavior based on the enum scattered throughout classes, the intent can be both obfuscated and duplicated. When adding a value to the enumeration, it's easy to miss a place that needs new behavior, introducing hard to detect bugs. The code is fragile in the face of changes. And because the state and associated behavior associated with the enumeration is mixed in with the class (or classes) using it, violation of the Single Responsibility Principle frequently is seen.

Refactoring to a data structure as mentioned above is a good first step. It helps address a number of the problems. But in today's object oriented world, there is a better way. In many (perhaps most) cases it's better to refactor to multiple classes, one for each value in the enumeration. Fortunately, this is fairly easily done.

Here's the original starting code:
enum SomeItemEnum = { one, two, three };
class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
First, a base class is created as an ancestor for the enumeration.
enum SomeItemEnum = { one, two, three };

class SomeItemEnumBase
{
}

class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
Then, each place where something needs to be done based on the value of the enumeration, a method is added to the class. If there is no default behavior for a method, it should be abstract, otherwise it should be virtual.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
A child class should be made for each value of the enumeration with the value specific behavior moved to the appropriate overridden method.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class SomeItemEnumOne : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemEnumTwo : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemEnumThree : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
Change the type of the class' variable from the enumeration to the base type.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class SomeItemEnumOne : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemEnumTwo : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemEnumThree : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnumBase someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
Instead of setting discrete values on the enumerated variable, now a class instance of the appropriate type can be set.

Old method:
someItemValue = SomeItemEnum.one;
someItemValue = SomeItemEnum.two;
New method:
someItemValue = new SomeItemEnumOne();
someItemValue = new SomeItemEnumTwo();
All the switch/if-else statements can now be changed to simple method calls that have polymorphic behavior based on the class type.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemBase
{
  abstract void ComplicatedCode();
}

class SomeItemOne : SomeItemBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemTwo : SomeItemBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemThree : SomeItemBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnumBase someItemValue;
  void SomeItemUser()
  {
    someItemValue.ComplicatedCode();
  }
}
And finally, the unused enumeration declaration can be removed.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class SomeItemEnumOne : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemEnumTwo : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemEnumThree : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnumBase someItemValue;
  void SomeItemUser()
  {
    someItemValue.ComplicatedCode();
  }
}
Now, when a new value is added, only places that actually care about the new value specifically (e.g. where its value is set) need to be touched. As a bonus, the compiler will complain about any unimplemented abstract functions, ensuring all required behavior is implemented.

Finally, admittedly, in this toy example, the final result is more complicated and obtuse than the original. If production code is as simple as this example, then it doesn't make sense to make this change. However, when class A is larger and more complex, the increased clarity and robustness of the code can be significant. In my experience, the latter is more typical than the former and so in general, my conclusion is enumerations border on evil.

Monday, February 24, 2014

Constant integer values and multi-language COM interop

I recently moved some code from a legacy C++ application into a COM library for more general use. The original code was duplicated a couple times in different C++ applications. Then a need arose to use this code in C#. To clean up both the duplication and make it available to .Net applications, we decided to untangle it from the original applications and move it into its own COM library.

One of the issues I ran into that took a bit to figure out involved constant values.

The old C code had several things defined as integers with associated constant definitions to handle bit-mapped values. Some might argue these should be converted to enumerations but we wanted to minimize changes to existing code structure and so decided to keep them as integer constants. The issue with this was how to move them to a common location for all COM clients to access.

It took a bit of research to find the answer as I didn't find a complete answer in one place. Hopefully this article will help fill that gap.

IDL files can have #define statements. This works for C code. The IDL compiler maps these as corresponding #defines in the corresponding intermediate .h files. The problem is when .Net creates an Interop assembly, they are ignored. This means they don't exist for use in .Net languages.

Next I used const definitions in the library. Again, this worked for C code but the Interop assembly in the .Net world did not have them.

I moved the const definitions into an interface. Still, C was perfectly happy but the Interop ignored them.

I search around some more. I ran into some forum discussions that indicated it was impossible.

Finally I found a reference to something that pointed me in the correct direction. IDL files can have the concept of modules, something I hadn't run into before. I put the const declaration in a module section in the IDL file. In the intermediate .h file for C++, these are simply constants and callers are still happy. But in this case, when the .Net Interop file is created they are not ignored. The Interop generator maps them to an abstract class named with the name of the module and the constants become static consts inside the class. This makes them available for any .Net user to access.

So, the IDL file ended up looking like this:
library SomeComLibrary
{
    module Constants
    {
        const DWORD UniqueValue = 0xFFFFFFFF;
    }
}
C's intermediate .h file looks like this:
#ifndef __Constants_MODULE_DEFINED__
#define __Constants_MODULE_DEFINED__
/* module Constants */
const DWORD UniqueValue = 0xffffffff;
#endif /* __Constants_MODULE_DEFINED__ */
And the .Net Interop file looks like this:
namespace SomeComLibrary
{
    public abstract class Constants
    {
        public const uint UniqueValue = -1;
    }
}
The solution ended up being quite easy, but finding it was a bit of a challenge.

Things I searched for trying to find a solution to this problem included:
  • idl const not in .net
  • idl const not in interop
  • idl const not in C# interop
  • midl keywords
  • idl const .net

Thursday, December 19, 2013

How to get Ref parameter values in RhinoMock

Mocking ref (and out) parameters with RhinoMock is a bit tedious. I ran into something that could possibly be considered a bug. At the very least it's not really expected behavior. The good news is I found a work-around. Hopefully this will help someone else (including perhaps my future self).

The basic problem is: Ref parameter values passed to mocked methods have the default values specified in the mock setup and not the values passed by the calling code.

The background

Take the following interface as an example.
interface ISomething
{
    void DoStuff(string a);
}
Now supposed this is mocked and some things need to be done based on the value of parameter a.

Normally this is easily done:
public void Initialize()
{
     mockSomething = MockRepository.GenerateStub();
     mockSomething.Stub(s => s.DoStuff(Arg.Is.Anything())
          .Do((Action)(arg =>
               {
                    // Perform general action with arg
                    
                    if (arg == "abc")
                    {
                         // Special case action for arg
                    }
               }));
}
Every time something calls mockSomething.DoStuff, the code passed to the Do method will be executed and the parameter arg will contain whatever value was passed to DoStuff. This is routine stuff for RhinoMocks and works as expected.

The setup

Now suppose the parameter for DoStuff was a ref.
interface ISomething
{
    void DoStuff(ref string a);
}
This is where things get a bit dicey. The interface is still mocked and some things still need to be done based on the value of parameter a. So, some minor syntax changes in the arguments constraints to handle the ref stuff for the compiler are done:
private delegate void DoStuffDelegate(ref string a);

public void Initialize()
{
     mockSomething = MockRepository.GenerateStub();
     mockSomething.Stub(s => s.DoStuff(ref Arg.Ref(Is.Anything(), string.Empty).Dummy)
          .Do((DoStuffDelegate)((ref arg) =>
               {
                    // Perform general action with arg
                    
                    if (arg == "abc")
                    {
                         // Special case action for arg
                    }
               }));
}

The problem

The above code compiles. And it runs. It just doesn't run correctly. The problem is that second parameter in the Arg<T>.Ref method. It indicates a return result for the the value. The problem is RhinoMocks sets the parameter value to the return result before calling Do's method. In other words, in this example, arg will always be string.Empty. The code in Do will never be called with the values sent to DoStuff by the original caller.

Looking at the call stack, I could see the original method call with the correct parameter values. Then it went into the RhinoMock and proxy code and then Do's method was called, clearly with the unexpected value.

Looking for solutions

Digging around, I found the WhenCalled method. This appears to be a bit earlier in the mock/proxy processing so I changed the test.
public void Initialize()
{
     mockSomething = MockRepository.GenerateStub();
     mockSomething.Stub(s => s.DoStuff(ref Arg.Ref(Is.Anything(), string.Empty).Dummy)
          .WhenCalled(invocation =>
               {
                    var arg = (string)invocation.Arguments[0];

                    // Perform general action with arg
                    
                    if (arg == "abc")
                    {
                         // Special case action for arg
                    }
               }));
}
Nope. This didn't work either. The value for Arguments[0] has already been set to the return value.

While searching around, I found other people asking about the same issue. In their cases they found alternative solutions based on constraints of when their methods were called and what values the parameters could have. With known values, constants can be used via hard coding. For example, instead of using Is.Anything() as above, Is.Equal("abc") can be used and the second parameter can be "abc". Everything is fine.

I was semi-successful with the special case by using this technique. But then I needed to do the special action and use Is.NotEqual("abc"). I ran into the same problem as with Is.Anything(). I didn't know the original value for arg.

The solution

Widening my search, I stumbled upon an old article by Ayende talking about the Callback method. He considered it for weird edge case parameter checking and indicated it shouldn't generally be used. As far as I could make out from his write-up, it's purpose is as an alternative to the Arg constraints when they aren't sufficient.

Having nothing to lose, I changed my code to give it a try:
private delegate bool DoStuffCallbackDelegate(ref string a);

public void Initialize()
{
     mockSomething = MockRepository.GenerateStub();
     mockSomething.Stub(s => s.DoStuff(ref Arg.Ref(Is.Anything(), string.Empty).Dummy)
          .Callback((DoStuffCallbackDelegate)((ref arg) =>
               {
                    // Perform general action with arg
                    
                    if (arg == "abc")
                    {
                         // Special case action for arg
                    }

                    return true;
               }));
}
Hurrah! This worked!

Since Callback's intent is to determine if the constraint on the stub is valid, it gets the original parameter values, rather than the return result.

And, yes, I realize I'm misusing the intent of Callback. But when nothing else works, you go with what does.

The conclusion

Since this met my needs, I stopped here. If this was a function rather than a method, I suppose a Do or Return method would have to be chained to the setup code after the Callback method in order to return a valid value for the stubbed function. Also note, if this stub should be ignored, then false can be returned from Callback instead of true. This would allow other stubs for the same method to be handled differently.

I'm not sure if this is a bug, intended behavior or an unconsidered edge case that has, at least to some, unexpected behavior. Both Out and Ref methods have the return result. It makes sense to be mandatory for Out, but I think it should be optional for Ref. I can see cases where you'd want to stub in a particular value all the time. The current syntax supports this well. But I can also see cases where it shouldn't be changed, at least by the Ref handling code. An overloaded version of Ref without the parameter would work well. In any case, I don't think it should be set before WhenCalled and Do are are invoked. At a minimum it should be after and better yet only if the original value hasn't been changed by WhenCalled or Do.

Well, that's my latest story regarding RhinoMocks. I hope it helps someone.

Friday, May 3, 2013

Extension methods are cool

You are creating a vocabulary, not writing a program. Be a poet for a moment.

-- Kent Beck

When Microsoft first introduced extension methods to C#, my first reaction was "eh". I viewed them as a novelty without much use. As time has worn on, I've come to appreciate them more and more. Their biggest win for me is to add features to basic system defined types and to fix what I consider deficiencies in the .Net libraries.

Static methods that take as a parameter an instance of the class they are defined in really annoy me. One frequent irritation: string.IsNullOrEmpty(). Every time I go to use this I always start writing the variable I want to test and then realize the method is static and have to go back and insert the "string.IsNullOrEmpty" at the front. This is simply one of many, many other similar methods spread throughout the framework with this.

Shortly after extension methods were added, one day while I was again grumbling at the IsNullOrEmpty implementation, I realized this would be an easy thing to fix. About five minutes later, after figuring out the syntax for extension methods, I had something like:
public static class StringExtensions
{
     public static bool IsNullOrEmpty(this string target)
     {
          return string.IsNullOrEmpty(target);
     }
}

Now I could write tests in the much more natural (for me) "if (someString.IsNullOrEmpty())..." format. Personally I find this much easier to read.

Flush with this success I immediately added another library function I remembered from Delphi that, in my opinion, helps with readability:

public static class ObjectExtensions
{
     public static bool IsAssigned(this object target)
     {
          return target != null;
     }
}

Instead of "if (someObj != null)..." I could now say "if (someObj.IsAssigned())..."

There is a real downside I with this though: Resharper does not recognize this as a test for null and reports a possible null value warning on subsequent accesses.

I'll admit, these aren't earth-shaking, industry-changing algorithms. But in day-in, day-out coding, I find the resulting code much easier to read.

Today I threw together a couple extensions to simplify work with Points, Rectangles and ranges:
public static Extensions
{
     public static Point Center(this Rectangle bounds)
     {
          return new Point((bounds.Left + bounds.Width) / 2, (bounds.Top + bounds.Height) / 2);
     }

     public static double DistanceTo(this Point p1, Point p2)
     {
          return Math.Sqrt(Math.Pow(p1.X - p2.X, 2) + Math.Pow(p1.Y - p2.Y, 2));
     }

     public static int ConstrainTo(this int constrainedValue, int min, int max)
     {
          return Math.Max(min, Math.Min(constrainedValue, max));
     }
}

With these, I transformed a method where the purpose was lost in all the notation to one where the purpose was eminently clear. Extension methods truly enable the craftsman to apply the opening quote from Kent Beck. They easily allow the developer to introduce, at the application level, domain vocabulary to system and other 3rd party classes.

Yes, extension methods are pretty cool!

Thursday, December 6, 2012

Apparent binding problems with NotSupportedException in PresentationFramework

I ran into this problem twice over the last couple weeks. Perhaps if I write an article, it'll help my future self (and just perhaps someone else) not spend so much time on it...

I put together a simple WPF form with a ListView. The ListLiew's View contained a GridView with columns bound to a POCO's properties. This was all done in XAML. In the constructor, I created an ObservableCollection to contain the objects and set it to the ListView's ItemsSource property. An event fired when interesting things happened. The event handler created the POCO object, set its properties and added it to the collection.

Everything should have worked.

However, when items were added to the collection, the debug window reported:
A first chance exception of type 'System.NotSupportedException' occurred in PresentationFramework.dll
Over the years, I've found data binding to be a challenge to get working. Part of the problem is it's mostly silent when it fails. So, data simply doesn't show up where it's expected with no indication as to why. Because of this, I assumed this is where the problem lay and spent significant time trying to figure out what was happening.

After quite a while I realized the event handler was called from a non-UI thread. I changed the line that added objects to the collection from:
ListViewDetails.Add(detailData);
to:
Dispatcher.Invoke((Action)(() =>
    ListViewDetails.Add(detailData)));
and everything worked properly. A simple solution once I realized what was wrong.

It'd be really nice if the debug window would output the exception message and not just the type. This would have told me immediately what was wrong and I wouldn't have spent so much time going down a dead-end road.

Wednesday, October 31, 2012

Sandcastle Help File Builder and error MSB4062 on Team 2008 build agents

I recently had to add a Sandcastle Help File Builder task to our automated build process. Our product consists of both .Net 3.5 projects and native C++ projects. It compiles under Visual Studio 2008 with Team 2008 build agents.

The latest version of Sandcastle is built against .Net 4.0, so, as part of its installation, I also installed the .Net 4.0 runtime. Everything worked fine in the GUI but when the build agent tried to run, it failed with:
error MSB4062: The "SandcastleBuilder.Utils.MSBuild.BuildHelp" task could not be loaded from the assembly C:\Program Files\EWSoftware\Sandcastle Help File Builder\\SandcastleBuilder.Utils.dll. Could not load file or assembly 'file:///C:\Program Files\EWSoftware\Sandcastle Help File Builder\SandcastleBuilder.Utils.dll' or one of its dependencies. This assembly is built by a runtime newer than the currently loaded runtime and cannot be loaded. Confirm that the <UsingTask> declaration is correct, and that the assembly and all its dependencies are available.

The UsingTask declaration appeared correct and all the dependencies seemed to be in order. I searched for quite a while and found several places purporting to have answers, but none of them worked. As I expanded my search, I finally happened upon an InfoSupport article about building VS2010 solutions with the 2008 build agent. This article described a means to fix a problem with a similar error message and, even though it wasn't related to Sandcastle, I decided to give it a try.

Short answer: it worked!!

The fix feels more like a hacky workaround than a good solution, but absent anything better, and given the fact that it allows the build to actually function properly, it currently stands.

The solution is to change a setting in a .Net configuration file.
  1. Edit %Program Files%\Microsoft Visual Studio 9.0\Common7\IDE\PrivateAssemblies\TFSBuildService.exe.config.
  2. Find the line that assigns a value to the key MSBuildPath.
  3. Change the value from an empty string to the directory of the .Net runtime that should be used, e.g. %WINDOWS%\Microsoft.NET\ Framework\v4.0.30319.
  4. Save the file.
  5. Either stop/stop the TFS build agent service or reboot the machine.

I hope that helps someone else.

Friday, June 22, 2012

List all values of an enumeration

I'm probably late to the party on this one, but I recently discovered a cool, handy little static method tucked away in the Enum class: GetValues. It simply returns all the values of an enum type. This enables nifty things like:
public enum TestEnum
{
    Value1,
    Value2,
    Value3,
    Value4
};

public void ShowEnumValues()
{
    Enum.GetValues(typeof (TestEnum))
        .Cast<TestEnum>()
        .ToList()
        .ForEach(e => Console.WriteLine(e));
}
With this, any changes to TestEnum automatically get picked up at run-time without any additional changes to the rest of the code.

Last year, I wrote about using a dictionary to facilitate converting between an enum and human readable text. To adapt the code presented then to the above example, the following will convert all values in the enum to human readable text.
private IDictionary<TestEnum, string> enumToString =
    new Dictionary<TestEnum, string>
        {
            {TestEnum.Value1, "Value one"},
            {TestEnum.Value2, "Value two"},
            {TestEnum.Value3, "Value three"},
            {TestEnum.Value4, "Value four"}
        };

public void ShowEnumStrings()
{
    Enum.GetValues(typeof(TestEnum))
        .Cast<TestEnum>()
        .ToList()
        .ForEach(e => Console.WriteLine(enumToString[e]));
}

In the previous article, I lamented the fact that this C# solution, unlike the Pascal variant, does not provide a reliable means to check that all the values of the enumeration have a corresponding entry in the dictionary. Because of strong typing with generics, I know I can't have any thing that's not part of the enumeration in the dictionary, but I can't know I have all the values of the enumeration in the dictionary. In other words, I know I can't have any invalid values, but I can't know I have all the values.

Until I found this method. Now I can at least write an easy test case for this. While it's not quite as good as a compile time check, it does give an early, reliable means of being alerted to the problem.
[TestMethod]
public void EnsureAllEnumValuesAreInDictionary()
{
    Assert.IsTrue(
        Enum.GetValues(typeof(TestEnum))
            .Cast<TestEnum>()
            .All(type => EnumToString.ContainsKey(type)),
        "Enum value missing from enumToString dictionary");
}

Since variables of this type typically do not change from instance to instance, they can be made static so they're only instantiated once. And since many times they are only used inside a single class, they can be made private. In these cases having a unit test might be problematic, not being able to test a private field. To solve this, code similar to the above test can be put in a static constructor, throwing an exception if there's a mismatch.
private static readonly IDictionary<TestEnum, string> EnumToString =
    new Dictionary<TestEnum, string>
        {
            {TestEnum.Value1, "Value one"},
            {TestEnum.Value2, "Value two"},
            {TestEnum.Value3, "Value three"},
            {TestEnum.Value4, "Value four"}
        };

static UnitTest1()
{
    if (!Enum.GetValues(typeof (TestEnum))
             .Cast<TestEnum>()
             .All(type => EnumToString.ContainsKey(type)))
        throw new InvalidOperationException(
            "Enum value missing from enumToString dictionary");
}

Finally, if this needs to be done much, making a generic helper function will clean-up code, hiding all the casting and repetition of the type.
private static IEnumerable<T> GetEnumValues<T>()
{
    return Enum.GetValues(typeof (T)).Cast<T>();
}

[TestMethod]
public void EnsureAllEnumValuesAreInDictionary1()
{
    Assert.IsTrue(
        GetEnumValues().All(type => EnumToString.ContainsKey(type)),
        "Enum value missing from enumToString dictionary");
}

All in all, I'm pleased with finding this little nugget.

A complete VS2008 solution containing the code examples above is available on github.

Hope this helps someone else.

Monday, June 4, 2012

Functional FizzBuzz in C#

Back when Jeff Atwood first wrote about FizzBuzz, I implemented it a number of different ways in Delphi. It was an interesting exercise; I think I came up with around five different ways to implement it. I also did some performance testing and was surprised by the results. Perhaps someday I'll try to find that code and publish it. Over the years since, I've used it, or variations, in interviews and have been surprised at how many applicants it weeded out.

Recently I ran across an article listing several ways to do it in various functional languages and wondered how functional I could code it in C#. So, I fired up Visual Studio and created a new console project. I didn't think my first attempt was too bad.
namespace FunctionalFizzBuzz
{
  class Program
  {
    static string FizzBuzzResult(bool showFizz, bool showBuzz, int val)
    {
      return showFizz && showBuzz
        ? "FizzBuzz"
        : showFizz
          ? "Fizz"
          : showBuzz
            ? "Buzz"
            : val.ToString(CultureInfo.InvariantCulture);
    }

    static string GenerateFizzBuzz(int start, int end)
    {
      return Enumerable
        .Range(start, end)
        .Aggregate(new StringBuilder(), (seed, val) =>
          {
            seed.AppendLine(FizzBuzzResult(val % 3 == 0, val % 5 == 0, val));
            return seed;
          })
        .ToString();
    }

    static void Main(string[] args)
    {
      Console.WriteLine(GenerateFizzBuzz(1, 100));
    }
  }
}

However, there were a couple things I didn't like about it. First, returning the whole result as a string seemed a bit overkill and intuitively not too memory friendly. For purposes of displaying 100 lines, it's not too bad, but for a more general use case, it's not good for composition. Second, the use of Aggregate bothered me a bit. How it works is not obvious to most people when first seeing it; it's use is a bit obtuse. And concatenating a bunch of strings just felt wrong.

So, I replaced the string result with an IEnumerable. This allowed changes which I think expresses the intent better. First, the Aggregate could be be changed to a Select, clarifying the GenerateFizzBuzz method. Second, the call to Console.WriteLine to generate each output line could be passed as a parameter to an iterator function. This is an example of how the new method makes it easier to use the output for different things. Another example is to count how many "Fizz" lines there are.

static IEnumerable GenerateFizzBuzz(int start, int end)
{
  return Enumerable
    .Range(start, end-start+1)
    .Select(val => FizzBuzzResult(val % 3 == 0, val % 5 == 0, val));
}

static void Main(string[] args)
{
  var result = GenerateFizzBuzz(1, 100).ToList();
  result.ForEach(Console.WriteLine);
  Console.WriteLine("Number of Fizzes: {0}", results.Count(l => l == "Fizz"));
}

With the GenerateFizzBuzz method cleaned up some, my attention turned to the FizzBuzzResult method. I wasn't terribly happy with the multiple nested conditional statement. While functional, it seemed more computational than declarative to me. What it did was a bit opaque. I like the parameter pattern matching available in the more functional languages and wondered how I could do something similar. I couldn't think of a way to let the compiler do it and regular expressions (the only pattern matching I can think of in the .Net libraries) only works on strings. However, it seemed pretty straight forward using a data structure.

I created a dictionary with a boolean key and dictionary value. The nested dictionary also had a boolean key but with a string value. I initialized these with the four possible values. The case which should return the numeric value was set to null. Then then the result was simply the lookup value of the dictionaries unless it returned null, in which case it was the passed in value, converted to a string.

private static readonly Dictionary<bool, Dictionary<bool, string>>
  FizzBuzzMap =
    new Dictionary<bool, Dictionary<bool, string>>
      {
        {true, new Dictionary<bool, string> {{true, "FizzBuzz"}, {false, "Fizz"}}},
        {false, new Dictionary<bool, string> {{true, "Buzz"}, {false, null}}}
      };

static string FizzBuzzResult(bool showFizz, bool showBuzz, int val)
{
  return FizzBuzzMap[showFizz][showBuzz] ?? val.ToString(CultureInfo.InvariantCulture);
}

I liked the way FizzBuzzResult looked, but the initialization code for the dictionaries was a bit ugly to my eye. I tried changing it to a single dictionary containing an array of booleans as the key.

private static readonly Dictionary<bool[], string>
  FizzBuzzMap =
    new Dictionary<bool[], string>
      {
        {new[] {true, true}, "FizzBuzz"},
        {new[] {true, false}, "Fizz"},
        {new[] {false, true}, "Buzz"},
        {new[] {false, false}, null},
      };

static string FizzBuzzResult(bool showFizz, bool showBuzz, int val)
{
  return FizzBuzzMap[new [] {showFizz, showBuzz}] ?? val.ToString(CultureInfo.InvariantCulture);
}

Ah, this looked much nicer. But... it raised a key not found exception. Oops. Fail. I hypothesized the key comparison during lookup looked at the arrays as objects, rather than the values they contained, and so raised the exception. I quickly wrote a comparison class for the dictionary and passed in an instance of it.

internal class BoolArrayComparer : IEqualityComparer
{
  public bool Equals(bool[] x, bool[] y)
  {
    if (x.Length != y.Length)
      return false;
    return !x.Where((t, i) => t != y[i]).Any();
  }

  public int GetHashCode(bool[] obj)
  {
    return obj.Aggregate(0, (seed, val) => seed + val.GetHashCode());
  }
}

Sure enough, it now worked properly. So the initialization looked pretty, but at the expense of having to have this extra, ugly class floating around spoiling things. Reflecting on the prior solution with the arrays, I realized structures have slightly different semantics and might work better with comparison. I changed the boolean array to a two member structure.

internal struct FizzBuzzKey
{
  internal bool showFizz;
  internal bool showBuzz;
};

private static readonly Dictionary
  FizzBuzzMap =
    new Dictionary
      {
        {new FizzBuzzKey {showFizz = true, showBuzz = true}, "FizzBuzz"},
        {new FizzBuzzKey {showFizz = true, showBuzz = false}, "Fizz"},
        {new FizzBuzzKey {showFizz = false, showBuzz = true}, "Buzz"},
        {new FizzBuzzKey {showFizz = false, showBuzz = false}, null},
      };

static string FizzBuzzResult(bool showFizz, bool showBuzz, int val)
{
  return FizzBuzzMap[new FizzBuzzKey {showFizz = showFizz, showBuzz = showBuzz}] ?? val.ToString(CultureInfo.InvariantCulture);
}

And this worked without requiring a Comparer class. But it's quite a bit more verbose than the array notation; I really liked the succinctness of the array notation. Then I remembered structs can have constructors. I added one and this helped cut down the verbosity quite a bit.

So, in the end I finished this little exercise with the following.

namespace FunctionalFizzBuzz
{
  internal class Program
  {
    internal struct FizzBuzzKey
    {
      internal FizzBuzzKey(bool showF, bool showB)
      {
        showFizz = showF;
        showBuzz = showB;
      }

      internal bool showFizz;
      internal bool showBuzz;
    };

    private static readonly Dictionary
      FizzBuzzMap =
        new Dictionary
          {
            {new FizzBuzzKey (true, true), "FizzBuzz"},
            {new FizzBuzzKey (true, false), "Fizz"},
            {new FizzBuzzKey (false, true), "Buzz"},
            {new FizzBuzzKey (false, false), null},
          };

    static void Main(string[] args)
    {
      var results = Enumerable
        .Range(90, 11)
        .Select(val => FizzBuzzResult(val%3 == 0, val%5 == 0, val))
        .ToList();
      results.ForEach(Console.WriteLine);
      Console.WriteLine("Number of Fizzes: {0}", results.Count(l => l == "Fizz"));
    }

    private static string FizzBuzzResult(bool showFizz, bool showBuzz, int val)
    {
      return FizzBuzzMap[new FizzBuzzKey(showFizz, showBuzz)] ?? val.ToString(CultureInfo.InvariantCulture);
    }
  }
}

Final thoughts

One solution I considered during this process was to map the four values of booleans to integers and do a lookup after a conversion. Intuitively, I didn't like the approach because it added an extra level of abstraction. Levels of abstraction can be good if they clarify. However, in this case, I thought it would simply obfuscate. In spite of this, out of desire to be complete, I threw this implementation together too.

private static readonly Dictionary
  FizzBuzzMap =
    new Dictionary()
      {
        {3 /* true, true */, "FizzBuzz"},
        {2 /* true, false */, "Fizz"},
        {1 /* false, true */, "Buzz"},
        {0 /* false, false */, null},
      };

static string FizzBuzzResult(bool showFizz, bool showBuzz, int val)
{
  return FizzBuzzMap[(val%3 == 0 ? 1 : 0) + (val%5 == 0 ? 1 : 0)] ?? val.ToString(CultureInfo.InvariantCulture);
}

But it didn't work. It emitted "Buzz" in places it should have output "Fizz." But not all the time. It took me a minute or two to find it.

Can you spot the error?


The fact that 1) this was the first bug I'd produced during this exercise, 2) it wasn't immediately obvious what the problem was and 3) the need for the comments in the setup, all confirmed my suspicion this is not a good solution.

Two other things I vacillated over was whether FizzBuzzResult needed to be a separate routine and, if so, what its parameters should be. At times I'd roll it into the main routine and other times I'd pass in just the numeric value and yet other times I'd pass in the three parameters as shown. I concluded that a toy exercise like this is too small to really have a clear winner in this debate. Additional use cases provided by a larger application would probably make one of these various options a clear winner.

After going through this process, I did a web search on "C# functional FizzBuzz" and found a number of solutions including this interesting take.

Monday, April 2, 2012

There and back again, again: Any resource's tale

Or, how to generically set and restore state when an exception is raised

I've written twice before (here and here) about making cursor changes more robust with the using statement in C#. These allow states to be reset to correct values regardless of whether or not an exception is raised during some processing. As promised at the end of the last article, today I explain how to use lambdas to easily protect any type of "resource" without having to create a new class or sprinkle your code with try/finally blocks.

To set things up and provide a bit of context, the other day in a code review, I found this:

someFlag = true;
someStartStateMethod();
// Do some work
someResetStateMethod();
someFlag = false;

Over the years, I've seen this pattern repeated, in different domains and different languages and not necessarily limited to setting a flag. Whenever I do, I ask "if an exception happens anywhere in 'do some work,' should the state be reset?" I won't say the answer should always be "yes," but I've not yet found a "no." And this case was no exception.

Since some of the places this was done was nested, I wanted a way to clean all this up that didn't involve adding try/finally blocks. Each try/finally would add another level of indentation; it would get pretty deep pretty fast. Also, it would clutter up the code with noise that didn't really have anything to do with the work needing to be done. In other words, I didn't want this:

someFlag = true;
try
{
   someStartStateMethod();
   try
   {
     // Do some work
   }
   finally
   {
      someResetStateMethod();
   }
}
finally
{
   someFlag = false;
}

In my last article, I separated the code actually handling the protection from the code controlling the invocation of that protection by using inheritance. I created a base abstract class to handle the general IDisposable pattern and descendent classes to handle the actual protection. In that article, I showed a class to handle changing a cursor and then changing it back to the current value.

I quickly realized this code was simply another example of that pattern. I could do the same thing as earlier and put the someStartStateMethod() and someResetStateMethod() in a descendant class. If I followed that pattern, the code under review could be something more along the lines of:

using(new ChangeSomeFlag())
using(new ChangeSomeStateMethod())
{
   // Do some work
}

This is somewhat better.

However, in this case, creating a new class for each thing needing protection didn't seem too appealing. There would be a lot of small special purpose classes, some of which might only be used once. It seemed like a sledgehammer to drive a brad. For this code, I wanted to emphasize composition over inheritance.

Enter lambdas. Because lambdas allow you to treat code as data, they allow easy composition. The LINQ and Rx libraries are great examples. They provide many generic methods (e.g. Where, Select, Any) that have the ability to compose in functionality through the use of lambdas to work on specific types for specialized purposes.

In this particular case, lambdas can be used to contain the code that handles the protection and passed to a class whose sole job is to handle the invocation of that code at the appropriate time. Rewriting the previous example with a new, as yet undefined ResourceProtector class, its use looks like:

using(new ResourceProtector(() => someFlag = true, () => someFlag = false))
using(new ResourceProtector(() => someSetStateMethod(), () => someResetStateMethod()))
{
   // Do some work
}

In this code, two expressions are passed to the constructor of the ResourceProtector class that handle the saving and restoring of state. Here is a single class that can be reused over and over in different situations to protect different types of resources. I think this is a beautiful example of the single responsibility principle combined with the power of lambda expressions to handle composition.

public class ResourceProtector : IDisposable
{
   private bool Disposed { get; set; }
   private Action EndProtection { get; set; }

   public ResourceProtector(Action startProtection, Action endProtection)
   {
      Disposed = false;
      EndProtection = endProtection;
      startProtection();
   }

   public void Dispose()
   {
      DisposeInt();
      GC.SuppressFinalize(this);
   }

   private void DisposeInt()
   {
      if (Disposed)
         return;

      EndProtection();
      Disposed = true;
   }

   ~ResourceProtector()
   {
      DisposeInt();
   }
}

If you have any questions, suggestions or comments, please feel free to leave them below. Until next time, subscribe to the Twitter feed to get notified of future articles and hear about other development related things.

Wednesday, March 21, 2012

7 ways to remove duplicate strings from a list (with timing results)

Recently there was a question on CodeProject about the fastest way to remove duplicate strings from a List. There were a bunch of alternative solutions presented, but no comparison to see what was faster. This post will show each solution and time them in a standard way to determine which actually answers the question.

About the test

Test setup creates a list of 1,000 random strings all 50 characters long. This is the expected result. It then creates duplicates for every other item and then for every 5th item. A list containing duplicates is created from the expected results and the duplicates. Note that for the even 5th items, there are actually two duplicates in the final list.

The core test method takes the list of duplicates, sets up a stopwatch and then calls the method to remove duplicates 1,000 times, each time passing in a new copy of the duplicate list. Since one of the methods under test changes the input list, this causes each iteration to work on the same, pristine input list. The elapsed time from the stopwatch is output and assertions are in place to make sure the method under test does what's expected.

About the methods under test

I've taken the code presented in the original thread and made slight modifications to make them all conform to the same interface. Some of them were code snippets or descriptions so I made my best guess as to the authors' intent.

1. The original poster's solution

public static IList LoadUniqueList1(List doubledList)
{
   var uniqueList = new List();
   foreach (var item in doubledList)
   {
      var x = true;
      foreach (var compare in uniqueList)
      {
         if (item == compare)
         {
            x = false;
         }
         if (!x) break;
      }
      if (x)
         uniqueList.Add(item);
   }
   return uniqueList;
}

2. Same as above, except substituting Contains for the second loop

public static IList LoadUniqueList2(List doubledList)
{
   var uniqueList = new List();
   foreach (var item in doubledList)
   {
      if (!uniqueList.Contains(item))
         uniqueList.Add(item);
   }
   return uniqueList;
}

3. Using a SortedList

public static IList LoadUniqueList3(List doubledList)
{
   var uniqueList = new SortedList();
   foreach (var item in doubledList)
   {
      if (!uniqueList.ContainsKey(item))
         uniqueList.Add(item, item);
   }
   return uniqueList.Values;
}

4. Using a HashSet

public static IList LoadUniqueList4(List doubledList)
{
   return new HashSet(doubledList).ToList();
}

5. Using LINQ Distinct

public static IList LoadUniqueList5(List doubledList)
{
   return doubledList.Distinct().ToList();
}

6. Using LINQ double iteration

public static IList LoadUniqueList6(List doubledList)
{
   var uniqueList = new List();
   uniqueList.Add(
      (from itm in uniqueList
      from compareItem in doubledList
      where itm == compareItem
      select compareItem).ToString());
   return uniqueList;
}

7. Sort and then build filtered list

public static IList LoadUniqueList7(List doubledList)
{
   var uniqueList = new List();
   doubledList.Sort();
   string lastEntry = string.Empty;
   foreach (string str in doubledList)
   {
      if (str != lastEntry)
      {
         uniqueList.Add(str);
         lastEntry = str;
      }
   }
   return uniqueList;
}

Timing results

The table below shows the elapsed time results for three test runs and the average elapsed time. The last column is how the strategy performed.

Before looking at the results, ask yourself "Which are the top two best performers?"


Comments about either the test methodology or results can be left below.

Test project available here.

Monday, March 12, 2012

There and back again, again: Several resource's tale

Or, how to safely change a cursor or other resource

In a previous article, I presented a simple class that utilized the IDisposable pattern to set the cursor to a specific value and then automatically reset it to the initial state when some work was accomplished. This simple class had two limitations: it only worked with WPF and it only worked on cursors.

To overcome these limitations, I refactored this to be more general. I changed the context to a generic type and added an abstract RestoreState method that's called when the class is either destroyed (preferred) or the finalizer is called. This allows all the basic plumbing regarding the IDisposable pattern to be contained in one class.

using System;

namespace ResourceProtection
{
    public abstract class ResourceChangeHandler : IDisposable
    {
        protected TContext Context { get; set; }
        private bool Disposed { get; set; }

        protected ResourceChangeHandler(TContext context)
        {
            Disposed = false;
            Context = context;
        }

        public void Dispose()
        {
            DisposeInt();
            GC.SuppressFinalize(this);
        }

        protected abstract void RestoreState(TContext context);

        private void DisposeInt()
        {
            if (Disposed)
                return;

            RestoreState();
            Disposed = true;
        }

        ~ResourceChangeHandler()
        {
            DisposeInt();
        }
    }
}

The beauty of this is it can be used as the base class for specific needs. The function of actually changing a resource is split out from the framework needed to manage the change. Here are three examples: one each for a WPF cursor changer, a WinForms cursor changer and a TreeView BeginUpdate/EndUpdate handler.

How to change a WPF cursor

public class WpfChangeCursor : ResourceChangeHandler{
    private Cursor OriginalCursor { get; set; }

    public WpfChangeCursor(FrameworkElement element, Cursor newCursor)
            : base(element)
    {
        OriginalCursor = Context.Cursor;
        Context.Cursor = newCursor;
    }

    protected override void RestoreState()
    {
        Context.Cursor = OriginalCursor;
    }
}

To use this, instantiate an instance of WpfChangeCursor in a using block, passing in the element who's cursor should be changed and the new value for the cursor. In this case, the context for the base class is the element containing the cursor to be changed. The descendant class has the responsibility to remember the current value and changing the cursor.

How to change a WinForm cursor

public class WinFormChangeCursor : ResourceChangeHandler{
    private Cursor OriginalCursor { get; set; }

    public WinFormChangeCursor(Cursor newCursor)
            : base(Cusor.Current)
    {
        Cursor.Current = newCursor;
    }

    protected override void RestoreState()
    {
        Cursor.Current = Context;
    }
}

This class is used by instantiating an instance of WinFormChangeCursor in a using block with the new value of the cursor passed to it. This is a little different from the WPF version since there is only one cursor for the application, rather than for each element. In this case, the context passed to the base is the current cursor value which is then used by RestoreState to put things back.

How to call BeginUpdate and EndUpdate safely

public class TreeViewUpdate : ResourceChangeHandler{
    public TreeViewUpdate(TreeView treeview)
            : base(treeview)
    {
        Context.BeginUpdate();
    }

    protected override void RestoreState()
    {
        Context.EndUpdate();
    }
}

And finally, here's a class to make sure EndUpdate is called for every BeginUpate on a TreeView. Like the WPF cursor changer above, the control is passed in as the context for the ancestor class. BeginUpdate is then called on the context. When the using block this class is controlled by finishes, the EndUpdate in RestoreState is called, again on the same context as the BeginUpdate.

This technique provides for a level of separation between the framework needed to safely do and undo things from the actual work of doing and undoing them. It works well for certain classes of problems, specifically fixed things that happen over and over again. The examples above of doing repetitive things on framework level items are good examples.

Sometimes though, one wants this type of functionality without the overhead of creating a new class each time. In the next article, I'll present a method of composing this type of behavior in a more adhoc manner.

Until then, subscribe to the Twitter feed to get notified of future articles and hear about other development related things. And if you have any questions, suggestions or comments, please feel free to leave them below.

Thursday, July 14, 2011

LINQ and set notation

So I learned something today.

It's been a long time since I had any classes dealing with sets. I vaguely remember discussions including Venn diagrams and odd notation that didn't have any relevance to any other context that I was familiar with, but they were also pretty cool to study and get to know. However, they're the types of things that have a highly technical, mathematically rigorous side to them and also the day-to-day intuitive side that I use. Not needing it on a regular basis, the finer points of set algebra quickly sink into the background and aren't easily recalled.

Intersection
Today I needed to do some set manipulation. I knew LINQ had some set related methods in it, such as Intersect which gives only the items that are in both lists.

Union
And I knew it had the Union that gives all the items in both lists, without duplicates.

Needing to find the differences between two lists, I suspected there was something to do that easily. I had two string lists and needed the items from both lists that were only in one of the two lists. In other words, I needed the inverse of Intersect. First I looked at the options in the code completion list. Nothing jumped out at me. I couldn't recall the technical name so I did a web search for "LINQ intersect inverse" and found the Except method. The description sounded promising but, when I tried it, I didn't get what I expected. It only gave the things that were in the first list but not in the second list. In other words, it did not include the things in the second list that weren't in the first. Figuring the people who wrote these methods probably knew something about what they were doing, I went digging a little deeper.

The next web search was for "set intersect inverse" that led me to a Wikipedia page on set intersection. Scrolling to the bottom "see also" section, I saw Complement. Ah-ha! That rang a bell from the distant past. Clicking on that link confirmed that Complement was in fact the difference between two sets.

RelativeComplement
And this is where I started to learn something new: there are multiple types of differences in set theory. In general, the Complement refers to things not in a given set. First, there's the relative complement as implemented by LINQ's Except method. This gives the things from one list that are not in the other list. For example, if list1 = new List<string> {"a", "b", "c"} and list2 = new List<string>{"b", "c", "d"} then list1.Except(list2) will return "a".

AbsoluteComplement
Second, there's the absolute complement. This is the universe of all things not in a given set. Hopefully it's obvious that this can't be implemented in software.

SymmetricDifference
Finally, there is the symmetric difference. This is the name of what I needed. It's the list of things that are in only one of two given sets. Thinking in terms of set operations, it's the relative complement between the union of the sets and the intersection of the sets. In LINQ terms it's list1.Union(list2).Except(list1.Intersect(list2)). This got me what I want.

Now that I had a technical term to search on, out of curiosity I tried "symmetric difference LINQ". (It's always easier to find what you're looking for when you know the proper keywords.) This returned a link to a StackOverflow question that not only gave the answer I came up with but also pointed out there's a SymmetricExceptWith method on the HashSet class.

Curiosity now drove me to do a quick benchmark. Knowing LINQ has never been a speed demon, I assumed it probably wouldn't beat out the HashSet implementation. I threw together a quick console app that simply ran both the above LINQ query and HashSet.SymmetricExceptWith call on two short lists a million times and reported the elapsed times. I used three different types of input containers to see if that made much difference. Here's what I found...
Input typeUsing LINQUsing HashSet.SymmetricExceptWith
List<string>2.8 s1.0 s
LinkedList<string>2.9 s1.1 s
HashSet<string>2.7 s0.8 s

Friday, July 8, 2011

Local procedures from Delphi in C#

Pascal has this nifty feature where you can declare procedures inside of procedures. Something like:
procedure procA

    procedure procB
    begin
        // procedure B code here
    end;

begin
    // procedure A code here calling procB
end;

It's not the type of thing I used a lot, but there were times where it came in handy. It was a nice thing to have in the tool belt even if it wasn't pulled out too often. A couple times I have wanted to do this in C# and have resorted to other means.

Today while reading an article by John Cook about C++, I realized this is trivially easy to do in C#: simply use named lambdas. It's one of those things that's so obviously simple and easy, I don't know why it didn't occur to me before.

So, the above Pascal code could be written in C# like so:
public void procA()
{
    Action procB = () => 
        {
            // procedure B code here
        }

    // procedure A code here calling procB()
}

Thursday, May 26, 2011

There and back again: A Cursor's tale

I've been working on UI code recently and have run into this quite a bit:
Cursor = Cursors.Wait;
SomeLongRunningTask();
Cursor = Cursors.Default;
Pretty straight forward code. I see this type of thing a lot, not just in the C# code I'm working on now, but I've seen this in the past. In other languages. On other platforms. It's a common practice. The problem is it only works in some cases.

This will have problems if any method in the "long running task" does the same thing. The cursor will be set back to default prematurely. Of course, this is easily solved by saving the old value prior to setting the new one.
var oldCursor = Cursor;
Cursor = Cursors.Wait;
SomeLongRunningTask();
Cursor = oldCursor;
This is an improvement but will still fail if an exception is raised. This can be fixed with a try/finally.
var oldCursor = Cursor;
Cursor = Cursors.Wait;
try
{
    SomeLongRunningTask();
}
finally            
{
    Cursor = oldCursor;
}
Ok. So now we have some code that works reasonably well. The issue now is one of code maintainability and clarity. Every method that needs to change the cursor needs to add nine lines before it can do any work of its own. It's nine lines where bugs could hide. It's nine lines obscuring the real purpose of the method. There's got to be a better way.

Way back in my Delphi days, I had a small class which handled this chore for me. The constructor took a parameter containing the new cursor. The current cursor was saved in a class field, the cursor was changed and the constructor finished. The destructor simply set the cursor back to the saved value. In that environment, interfaces were reference counted and destroyed when the reference went to zero. The compiler took care of all the bookkeeping automatically. So I could create an instance of this class, store it in a local variable and, when the variable went out of scope, the object was destroyed, resetting the cursor. It worked really well. One line to change the cursor to whatever I wanted and it automatically reset to the original value when done. Pretty sweet.

Unfortunately, C# does not have any deterministic way of handling object lifetimes. This is one of my biggest frustrations with the language. The closest thing we have is the hack that is the using statement. Since this is the best we have, I have put together a class, shown below, similar to the one I had before. The constructor takes the new cursor value and control whose cursor should be changed, saves the current one and, in the Dispose method, resets it back. Now we can have three simple lines with one code block rather than the nine lines with two blocks. It's a big improvement although still not the single line like I'd prefer.

In use, it looks like this:
using ChangeCursor(this, Cursors.Wait)
{
    SomeLongRunningTask();
}
Arguably, this is a bit cleaner than even the first code snippet we started with.

Since this takes a FrameworkElement, it's WPF specific. It'd be easy enough to change the type to work with WinForms, if needed. The same technique could be used to handle other things where a state needs to be set to one value and then reset when some unit of work is done. For example, if using TreeViews with BeginUpdate/EndUpdate pairs.

Hope this helps someone.
using System;
using System.Windows;
using System.Windows.Input;

namespace CursorResourceProtection
{
    public class ChangeCursor : IDisposable
    {
        private FrameworkElement Context { get; set; }
        private bool Disposed { get; set; }
        private Cursor OriginalCursor { get; set; }

        public ChangeCursor(FrameworkElement context, Cursor newCursor)
        {
            Disposed = false;
            Context = context;
            OriginalCursor = context.Cursor;
            context.Cursor = newCursor;
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        protected virtual void Dispose(bool disposing)
        {
            if(Disposed)
                return;

            Context.Cursor = OriginalCursor;
            Disposed = true;
        }

        ~ChangeCursor()
        {
            Dispose(false);
        }
    }
}

Wednesday, May 11, 2011

Non-int array indexes from Delphi in C#

In Delphi, array indexes can be any type that resolves to an integer, not just integers. So for example, enum and bool types can be array indexes. This can be handy in many situations to reduce a if-then or switch construct to a simple assignment. A simple, contrived example:


MyEnum = (item1, item2, item3);
...
function enumToString(value: MyEnum): string
begin
switch(value)
case item1: result = "Item one";
case item2: result = "Item two";
case item3: result = "Item three";
end;
end;
...
SomeLabel.Caption = enumToString(value);
...

becomes something like this:


MyEnum = (item1, item2, item3);
enumToString[MyEnum]: string = ("Item one", "Item two", "Item three");
...
SomeLabel.Caption = enumToString[value];
...
Not only does it reduce code, it also reduces possible bugs. Consider what happens when a value is added to MyEnum. The first example will simply return an empty string creating a possible silent failure that hopefully will be caught when tests are run. The second will generate a compiler error, forcing the programmer to intentionally decide what to do.

In C#, array indexes must be one of the four integer types: int, uint, long or ulong. I really miss the ability to use bools and enums. While lamenting this lack as I looked at some code today similar to the original switch statement, I realized I could use a generic dictionary. The new code looks like this:

MyEnum = (item1, item2, item3);
enumToString = new Dictionary = {{item1, "Item one"}, {item2, "Item two"}, {item3, "Item three")};
...
SomeLabel.Contents = enumToString[value];
...
On one hand, it is a bit more wordy than the Delphi array; on the other, it's clearer how things are associated in the initializer. The biggest drawback is it doesn't have the compiler error when things are added to the enum. This is mitigated a bit by the runtime exception generated by a missing key; it's not quite as silent as just returning nothing in the original code. However, it still depends on good test coverage to find it.

Friday, October 23, 2009

Canceling application shutdown in WPF

I hate "Are you sure" type dialog boxes. Gratuitous use of them in software makes me want to do violence to whoever thought it was a good idea to put it in. That said, there are a few times where they make sense. For example, the application I'm working on now controls an instrument that may have some expensive, hard to replace, chemistry in it. Stopping the application while the instrument is in use may result in moderate financial loss, so in this case, it does make sense to alert the user to a possible undesired outcome and make sure they really want to proceed with the action.

The WPF Window class has a Closing event that fires when the window is closed. This event has an argument containing a Cancel property with a false default value. Setting this property to true inhibits the closing of the window. When this happens on the application’s MainWindow, this closing cancellation behavior applies to the application as a whole.

Some time ago, our application had code put in to pop up an “Are you sure?” style dialog and set the event's property as appropriate. The problem was it worked for some pathways through the code but not others. The lot fell to me to investigate. There are four ways to close the application: the Alt-F4 key, the task bar's Close option, the Close button in the window's title bar and an Exit menu option. All the methods worked except the Exit menu.

Digging into the code, the Exit method menu called the Shutdown method on the Application.Current object. This seemed like a reasonable thing to do and I went looking elsewhere for the problem. After spending time verifying there wasn't anything else odd in our code I came back to this method. On a hunch, I changed this to calling the Close method on Application.Current.MainWindow. This fixed the problem. I'm not sure why, but apparently the Shutdown method does something to inhibit the normal message processing of open windows.

I know this is typical behavior and it fixes my application for now, but from a design standpoint, I'm not too sure I like this. It seems like application level shutdown code in the Window is at the wrong layer. The Application class has an Exit event that's analogous to the Window's Close event however, this does not allow canceling the operation. In my opinion, the Application class should have an Exiting event with a Cancel property on the argument, similar to the Window class' Closing event.

In any case, if your application does not call the Closing event as you expect, first check to see how it's shutdown.

(25-June-2012: Minor edit for clarity.)

Friday, August 14, 2009

How to display items from a collection in one line of a DataGrid

Typically, data structures map closely to their representation in the user interface and so most display widgets work pretty well in this situation. In WPF, DataContexts and bindings expect the relationship between their controls and the bound data to be fairly similar. Recently, I had a weird requirement that caused me trouble getting information on the screen due to a mismatch between the data model and the view.

Requirements

For good reasons, the data needed to be modeled in a master-detail relationship. For equally good reasons, the user wanted this displayed on a single line in a grid control. I believe the model to be correct; I'm not too sure about the user interface. I understand the user's reasons for wanting a single line, but I expect that over time and as new features are added, they will want a change in how things are presented. Regardless of what happens in the future, for today, this is the current requirement that I had to implement.

So, the data model looked something like this: 

And, with the assumption that there will not be more than one instance for any particular descendant type of TestItem in Property3, the goal was to have a screen that looked like this:

The problem

Setting up the data structures was no problem, nor was getting the master's data on the screen. I used a DataGrid from WPFTools and simple binding sufficed for Property1 and Property2. But when it came to PropertyA1 and PropertyB1 I ran into problems. Reading the documentation for binding paths, I found I could use the slash character to indicate items in a collection. Going down this trail, I tried setting the paths for the last two columns to Property3/PropertyA1 and Property3/PropertyB1. This sort of worked:
As shown above, the correct data is displayed, but only for the first item in the list. If the order of things in the list is changed, the data changes appropriately. Through the binding path, I could find no way to select different items from the list for different columns in the grid. Perhaps it can be done and I didn't find how, but understandably, this is an odd thing to try to do and it's not a surprise if the framework doesn't support it. (See CollectionBinding1 in the source code.)

Solution: IValueConverter

I thought about trying to put the data in the cells directly without binding and, while searching how to do this, was reminded of IValueConverter. From the examples, the standard way of using this is to simply convert between types or to control formatting. However, I realized I could create one that takes the entire list and pulls out from it a specific field.

First, I changed the binding for the two problematic columns to be just Property3. Then I created a new converter class for each property, and finally instantiated and assigned them to the columns' bindings. This resulted in a one-to-one relationship between converter classes and each property. The converters iterated the list until an entry of its expected type was found and then returned the field value it knew about. A converter for each property: ugly, but it worked! (See CollectionBinding2 in the source code.)

After proving the concept, I looked to clean it up a bit. The only differences between the classes was the type it was looking for and the property it returned. I realized these two things could be parameterized. I could change the converter to a generic class and then use the generic type in the loop. Also, I could pass a lambda expression to the constructor that would later be used in the loop to get the property value if the given type was found. I made these relatively minor changes, eliminating the multiple classes and making the strategy effective for my real world scenario. (See CollectionBinding3 in the source code.)

Final thoughts

Typically, converters are instantiated and referenced in the XAML code with just the implementation residing on the C# side. This works because the number of types are relatively small. My original implementation would probably work with this usage pattern, however, in the real world situation, with its one-to-one relationship between properties and converter classes, I would have an inordinate number of classes to have to manage. By moving the instantiation into C#, I can use a single class to handle the different types of objects in the list and the multiple properties in each type.

Source code for the three stages of development is available in a zip file here.

Any comments are welcome.

Friday, June 26, 2009

How to display tool tips in WPF DataGrid text column

Like many people before me, while using the DataGrid from the WPFtoolkit, I recently had a requirement to display text nicer than the default. First, the default is simply to truncate the text like so:

I needed the truncated text to have ellipses after it and have a ToolTip showing the entire text. Something like this:

I did a lot of searching the web and didn't find anything straight-forward and elegant. I found things that intimated using styles in XAML might work, although nothing directly addressing what I wanted to do. Furthermore, I found others saying XAML styles won't work at all for cell level changes along with some ugly hacks where, inside the LoadingRow method, they walked back up the VisualTree to change the cell style in code. My search was confounded by the fact that there are a number of different DataGrid components, three by Microsoft (WinForms, WPF and Silverlight) and several by other 3rd party vendors. All have issues doing this very basic thing and all with different workarounds.

After spending the better part of a day researching and testing various possible solutions, I found a comment on a Silverlight forum with the idea of using a DataGridTemplateColumn descendant to handle the new needs. I started to adapt the example code from Silverlight to WPF and thought, "There must be an easier way." Since the existing DataGridTextColumn already did much of what was in this example, I went off to look at its source code. While browsing through it, I found a protected virtual method called GenerateElement that apparently is a factory for the cells' display. On a hunch, I created a descendant of this column type and overrode this one method, changing the properties of the returned value as needed.

Amazingly, it worked the first time. So, to add my own workaround to the mix that's found on the web, here is the class I created:
public class DataGridToolTipTextColumn : DataGridTextColumn
{
    protected override FrameworkElement GenerateElement(DataGridCell cell, object dataItem)
    {
        var result = (TextBlock)base.GenerateElement(cell, dataItem);
        result.TextTrimming = TextTrimming.CharacterEllipsis;
        ApplyBinding(result, FrameworkElement.ToolTipProperty);
        return result;
    }

    // Copied from DataGridTextColumn because it's not protected there either. Seems like it should be.
    internal void ApplyBinding(DependencyObject target, DependencyProperty property)
    {
        var binding = Binding;
        if (binding == null)
            BindingOperations.ClearBinding(target, property);
        else
            BindingOperations.SetBinding(target, property, binding);
    }
}
The GenerateElement method simply calls the base class and typecasts the return value to a TextBlock. For the moment this is safe since the base class has the type hard coded. The TextTrimming property is then set for CharacterEllipsis and the ToolTip property is bound to the same value as the Text property. This class is twice as long as it needs to be because the ApplyBinding method that exists in the base class is marked internal rather than protected, so I simply made a copy of it for use here.

In my usage, this DataGrid is only to display information; editing is turned off. I haven't checked to see if this would cause any issues in edit mode.

Related material

In all this searching, I did find a couple good resources:
  • Samuel Moura has a 4 part series doing some pretty intensive DataGrid styling and templating: Introduction, Custom Styling, Playing with Columns and Cells and TemplateColumns and Row Grouping.
  • The Tranxition Developer Blog has a good article on adding dependency properties to the TextBlock type. Unfortunately, this doesn't seem to work for TextBlocks that are in another control's VisualTree.
  • This Silverlight thread is what gave me the idea for creating a descendant class.
  • I ran across this great little utility called Snoop to inspect the VisualTree of a running WPF application. I think I'd heard of it before in passing but had never looked into it. I don't know how many times I've written throw-away VisualTree walkers to dump information to the console. Now I'll never have to do that again.

Tuesday, May 12, 2009

WPF Colors, static classes and reflection in .Net

Today I wanted to know the definitions of the colors in WPF's static Colors class. A two-minute web search didn't turn anything up so I decided to write a quick and dirty little program to emit them. It should have been simple enough; I should have known better.

Using reflection, I wanted to cycle through each property of the Colors class and output its name and RGB value. To this end, I typed this into Visual Studio:
foreach (var prop in typeof(Colors).GetProperties())
{
    var propColor = (Color)prop.GetValue();
}

Then I hit a snag: GetValue wanted an instance of the class. However, Colors is static and I could not create an instance of it. In looking through the interface, I found GetConstantValue method. That sounded promising. I tried it. It compiled but threw an exception at run-time indicating it wasn't going to work.

I read the PropertyInfo documentation and didn't see anything more promising than GetValue. I searched the web some more but the only thing I could find talked about calling static methods on normal classes with instances of them created. I went back to the documentation where the description for the first parameter still said "The object whose property value will be returned." Reading on, the second parameter was the index value and said, "This value should be null for non-indexed properties."

Hmm... What happens if the first parameter is null?

Having nothing to lose, I tried it with both parameters passed as null. It compiled. A breakpoint set after the assignment and then inspection of propColor indicated it worked as expected. Cool!

I put a WrapPanel in the XAML file and named it ColorItems. Then I fleshed out my for loop:
foreach (var prop in typeof(Colors).GetProperties())
{
    var propColor = (Color)prop.GetValue(null, null);
    var desc = string.Format("{0} ({1})", prop.Name, propColor);
    Console.WriteLine(desc);
    ColorItems.Children.Add(new Grid
        {
            Width = 20,
            Height = 20,
            Margin = new Thickness(2),
            ToolTip = desc,
            Background = new SolidColorBrush(propColor)
        });
}

This gave me a nice little window with squares of color. Each square showed a tool tip with the name and color. Moreover, I had the table shown below in the Output window of the IDE.

Download the zipped project with compiled application.

ColorValueSample
AliceBlue#F0F8FF 
AntiqueWhite#FAEBD7 
Aqua#00FFFF 
Aquamarine#7FFFD4 
Azure#F0FFFF 
Beige#F5F5DC 
Bisque#FFE4C4 
Black#000000 
BlanchedAlmond#FFEBCD 
Blue#0000FF 
BlueViolet#8A2BE2 
Brown#A52A2A 
BurlyWood#DEB887 
CadetBlue#5F9EA0 
Chartreuse#7FFF00 
Chocolate#D2691E 
Coral#FF7F50 
CornflowerBlue#6495ED 
Cornsilk#FFF8DC 
Crimson#DC143C 
Cyan#00FFFF 
DarkBlue#00008B 
DarkCyan#008B8B 
DarkGoldenrod#B8860B 
DarkGray#A9A9A9 
DarkGreen#006400 
DarkKhaki#BDB76B 
DarkMagenta#8B008B 
DarkOliveGreen#556B2F 
DarkOrange#FF8C00 
DarkOrchid#9932CC 
DarkRed#8B0000 
DarkSalmon#E9967A 
DarkSeaGreen#8FBC8F 
DarkSlateBlue#483D8B 
DarkSlateGray#2F4F4F 
DarkTurquoise#00CED1 
DarkViolet#9400D3 
DeepPink#FF1493 
DeepSkyBlue#00BFFF 
DimGray#696969 
DodgerBlue#1E90FF 
Firebrick#B22222 
FloralWhite#FFFAF0 
ForestGreen#228B22 
Fuchsia#FF00FF 
Gainsboro#DCDCDC 
GhostWhite#F8F8FF 
Gold#FFD700 
Goldenrod#DAA520 
Gray#808080 
Green#008000 
GreenYellow#ADFF2F 
Honeydew#F0FFF0 
HotPink#FF69B4 
IndianRed#CD5C5C 
Indigo#4B0082 
Ivory#FFFFF0 
Khaki#F0E68C 
Lavender#E6E6FA 
LavenderBlush#FFF0F5 
LawnGreen#7CFC00 
LemonChiffon#FFFACD 
LightBlue#ADD8E6 
LightCoral#F08080 
LightCyan#E0FFFF 
LightGoldenrodYellow#FAFAD2 
LightGray#D3D3D3 
LightGreen#90EE90 
LightPink#FFB6C1 
LightSalmon#FFA07A 
LightSeaGreen#20B2AA 
LightSkyBlue#87CEFA 
LightSlateGray#778899 
LightSteelBlue#B0C4DE 
LightYellow#FFFFE0 
Lime#00FF00 
LimeGreen#32CD32 
Linen#FAF0E6 
Magenta#FF00FF 
Maroon#800000 
MediumAquamarine#66CDAA 
MediumBlue#0000CD 
MediumOrchid#BA55D3 
MediumPurple#9370DB 
MediumSeaGreen#3CB371 
MediumSlateBlue#7B68EE 
MediumSpringGreen#00FA9A 
MediumTurquoise#48D1CC 
MediumVioletRed#C71585 
MidnightBlue#191970 
MintCream#F5FFFA 
MistyRose#FFE4E1 
Moccasin#FFE4B5 
NavajoWhite#FFDEAD 
Navy#000080 
OldLace#FDF5E6 
Olive#808000 
OliveDrab#6B8E23 
Orange#FFA500 
OrangeRed#FF4500 
Orchid#DA70D6 
PaleGoldenrod#EEE8AA 
PaleGreen#98FB98 
PaleTurquoise#AFEEEE 
PaleVioletRed#DB7093 
PapayaWhip#FFEFD5 
PeachPuff#FFDAB9 
Peru#CD853F 
Pink#FFC0CB 
Plum#DDA0DD 
PowderBlue#B0E0E6 
Purple#800080 
Red#FF0000 
RosyBrown#BC8F8F 
RoyalBlue#4169E1 
SaddleBrown#8B4513 
Salmon#FA8072 
SandyBrown#F4A460 
SeaGreen#2E8B57 
SeaShell#FFF5EE 
Sienna#A0522D 
Silver#C0C0C0 
SkyBlue#87CEEB 
SlateBlue#6A5ACD 
SlateGray#708090 
Snow#FFFAFA 
SpringGreen#00FF7F 
SteelBlue#4682B4 
Tan#D2B48C 
Teal#008080 
Thistle#D8BFD8 
Tomato#FF6347 
Transparent#FFFFFF 
Turquoise#40E0D0 
Violet#EE82EE 
Wheat#F5DEB3 
White#FFFFFF 
WhiteSmoke#F5F5F5 
Yellow#FFFF00 
YellowGreen#9ACD32