Pattern for Creating a Simple and Efficient Value type

ErnieL picture ErnieL · Nov 7, 2011 · Viewed 9.7k times · Source

Motivation:

In reading Mark Seemann’s blog on Code Smell: Automatic Property he says near the end:

The bottom line is that automatic properties are rarely appropriate. In fact, they are only appropriate when the type of the property is a value type and all conceivable values are allowed.

He gives int Temperature as an example of a bad smell and suggests the best fix is unit specific value type like Celsius. So I decided to try writing a custom Celsius value type that encapsulates all the bounds checking and type conversion logic as an exercise in being more SOLID.

Basic requirements:

  1. Impossible to have an invalid value
  2. Encapsulates conversion operations
  3. Effient coping (equivalent to the int its replacing)
  4. As intuitive to use as possible (trying for the semantics of an int)

Implementation:

[System.Diagnostics.DebuggerDisplay("{m_value}")]
public struct Celsius // : IComparable, IFormattable, etc...
{
    private int m_value;

    public static readonly Celsius MinValue = new Celsius() { m_value = -273 };           // absolute zero
    public static readonly Celsius MaxValue = new Celsius() { m_value = int.MaxValue };

    private Celsius(int temp)
    {
        if (temp < Celsius.MinValue)
            throw new ArgumentOutOfRangeException("temp", "Value cannot be less then Celsius.MinValue (absolute zero)");
        if (temp > Celsius.MaxValue)
            throw new ArgumentOutOfRangeException("temp", "Value cannot be more then Celsius.MaxValue");

        m_value = temp;
    }

    public static implicit operator Celsius(int temp)
    {
        return new Celsius(temp);
    }

    public static implicit operator int(Celsius c)
    {
        return c.m_value;
    }

    // operators for other numeric types...

    public override string ToString()
    {
        return m_value.ToString();
    }

    // override Equals, HashCode, etc...
}

Tests:

[TestClass]
public class TestCelsius
{
    [TestMethod]
    public void QuickTest()
    {
        Celsius c = 41;             
        Celsius c2 = c;
        int temp = c2;              
        Assert.AreEqual(41, temp);
        Assert.AreEqual("41", c.ToString());
    }

    [TestMethod]
    public void OutOfRangeTest()
    {
        try
        {
            Celsius c = -300;
            Assert.Fail("Should not be able to assign -300");
        }
        catch (ArgumentOutOfRangeException)
        {
            // pass
        }
        catch (Exception)
        {
            Assert.Fail("Threw wrong exception");
        }
    }
}

Questions:

  • Is there a way to make MinValue/MaxValue const instead of readonly? Looking at the BCL I like how the meta data definition of int clearly states MaxValue and MinValue as compile time constants. How can I mimic that? I don’t see a way to create a Celsius object without either calling the constructor or exposing the implementation detail that Celsius stores an int.
  • Am I missing any usability features?
  • Is there a better pattern for creating a custom single field value type?

Answer

Reed Copsey picture Reed Copsey · Nov 7, 2011

Is there a way to make MinValue/MaxValue const instead of readonly?

No. However, the BCL doesn't do this, either. For example, DateTime.MinValue is static readonly. Your current approach, for MinValue and MaxValue is appropriate.

As for your other two questions - usability and the pattern itself.

Personally, I would avoid the automatic conversions (implicit conversion operators) for a "temperature" type like this. A temperature is not an integer value (in fact, if you were going to do this, I would argue that it should be floating point - 93.2 degrees C is perfectly valid.) Treating a temperature as an integer, and especially treating any integer value implicitly as a temperature seems inappropriate and a potential cause of bugs.

I find that structs with implicit conversion often cause more usability problems than they address. Forcing a user to write:

 Celsius c = new Celcius(41);

Is not really much more difficult than implicitly converting from an integer. It is far more clear, however.