Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Pattern for Creating a Simple and Efficient Value type

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?
like image 932
ErnieL Avatar asked Nov 07 '11 18:11

ErnieL


3 Answers

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.

like image 87
Reed Copsey Avatar answered Nov 02 '22 16:11

Reed Copsey


I think from a usability point of view I would opt for a type Temperature rather than Celsius. Celsius is just a unit of measure while a Temperature would represent an actual measurement. Then your type could support multiple units like Celsius, Fahrenheit and Kelvin. I would also opt for decimal as backing storage.

Something along these lines:

public struct Temperature
{
    private decimal m_value;

    private const decimal CelsiusToKelvinOffset = 273.15m;

    public static readonly Temperature MinValue = Temperature.FromKelvin(0);
    public static readonly Temperature MaxValue = Temperature.FromKelvin(Decimal.MaxValue);

    public decimal Celsius
    {
        get { return m_value - CelsiusToKelvinOffset; }
    }

    public decimal Kelvin 
    {
        get { return m_value; }
    }

    private Temperature(decimal temp)
    {
        if (temp < Temperature.MinValue.Kelvin)
               throw new ArgumentOutOfRangeException("temp", "Value {0} is less than Temperature.MinValue ({1})", temp, Temperature.MinValue);
        if (temp > Temperature.MaxValue.Kelvin)
               throw new ArgumentOutOfRangeException("temp", "Value {0} is greater than Temperature.MaxValue ({1})", temp, Temperature.MaxValue);
         m_value = temp;
    }

    public static Temperature FromKelvin(decimal temp)
    {     
           return new Temperature(temp);
    }

    public static Temperature FromCelsius(decimal temp)
    {
        return new Temperature(temp + CelsiusToKelvinOffset);
    }

    ....
}

I would avoid the implicit conversion as Reed states it makes things less obvious. However I would overload operators (<, >, ==, +, -, *, /) as in this case it would make sense to perform these kind of operations. And who knows, in some future version of .net we might even be able to specify operator constraints and finally be able to write more reusable data structures (imagine a statistics class which can calculate statistics for any type which supports +, -, *, /).

like image 21
ChrisWue Avatar answered Nov 02 '22 16:11

ChrisWue


DebuggerDisplay is useful touch. I'd add unit of measurements "{m_value} C" so you can immediately see the type.

Depending on target usage you may also want to have generic conversion framework to/from base units in addtion to concrete classes. I.e. store values in SI units, but be able to display/edit based on culture like (degrees C, km, kg) vs. (degrees F, mi, lb).

You may also check out F# measurement units for additioanl ideas ( http://msdn.microsoft.com/en-us/library/dd233243.aspx ) - note that it is compile time construct.

like image 23
Alexei Levenkov Avatar answered Nov 02 '22 15:11

Alexei Levenkov