Value Object encapsulating different units of measure

Question

I have a situation where I need to use person's height in different units. Some parts of the system use Inches/Feet some Meters/Centimeters.

I was thinking of a Height value object which would encapsulate different measurement untis, and upon creation calculate and store all supported measurement units and provide accessors to get each.

I end up with this

public class Height : IEquatable<Height>
    {
        private Height()
        { }

        public double InMeters { get; private set; }
        public double InFeet { get; private set; }
        public double InInches { get; private set; }

        public static Height FromCMeters(double heightInCms) => new Height
        {
            InMeters = (double) heightInCms / 100,
            InFeet = heightInCms * 30.48,
            InInches = heightInCms * 2.54
        };

        public static Height FromFoots(double feet) => new Height
        {
            InFeet = feet,
            InMeters = (feet * 30.48) / 100,
            InInches = feet * 12
        };

        public static Height FromInches(double inches) => new Height
        {
            InInches = inches,
            InFeet = inches / 12,
            InMeters = (inches * 2.54) / 100
        };

        public bool Equals(Height other)
        {
            if (ReferenceEquals(null, other)) return false;
            if (ReferenceEquals(this, other)) return true;
            return InMeters.Equals(other.InMeters);
        }

        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj)) return false;
            if (ReferenceEquals(this, obj)) return true;
            if (obj.GetType() != this.GetType()) return false;
            return Equals((Height) obj);
        }

        public override int GetHashCode() => InMeters.GetHashCode();
    }

I intentionally omitted addition/subsctraction to keep code small. The usage of Height value object:

var height = Height.FromCMeters(191);
// height.InMeters
// height.InInches
// height.InFeet

I have few doubts:

• If I need to support new unit of measure I would have to change the value object which I don't think is a good idea
• I tend to keep my value objects very small, this one seems to be doing too much for a value object (all those conversion logics)
• I think it keeps track of many values instead of a one, and it feels that all this conversion logic belongs to somewhere else

This Height VO is mostly going to be used to represent height. All operations are going to be creating height from different units (m/cm/feet/inch) and pass it around. Whoever receives it is going to use it just to retrieve height value in the unit it needs.

This simple usage makes me think I don't really need more involved design, where I would have simply Height VO (with single metric) and let another component do the necessary conversion.

What do you think ?

Answer 1

Some quick remarks:

• Don't pointlessly abbreviate: you gain nothing with "FromCMeters" or "heightInCms". Just use "Centimeters". I'd even argue that you shouldn't use the plural to express the height; I'd always say "I'm 189 centimeter". Same for "InMeters". (But I'm not a native English speaker.)

• There are several magic numbers used repeatedly: 30.48, 2.54, 12. IMHO these should be const with a meaningful name.

• I fully agree with Graipher's review: you should use one canonical unit and store its value in a private variable, and InMeter, InFeet and InInches should be "translations" of that unit (e.g. if you store it in cm, InMeter should divide the value by 100).

Answer 2

Instead of carrying around the conversion from each unit to each other unit, it is a better idea to decide on one canonical unit (this can be meters or inches, it does not matter).

Then to introduce a new unit, all you need to add is a FromUnit method and a InUnit method. These two methods just contain the conversion to and from that canonical unit.

This has the small drawback that you might need to do the conversion every time InUnit is accessed, instead of only once, but it saves you from having to write n! methods.

I don't know any C#, so unfortunately I don't know how exactly that would look like.

---

In Python, a language I am far more familiar with, this would look something like this:

class Height:
    meter_per_feet = 0.3048

    def __init__(self, value):
        self.in_meters = value

    @classmethod
    def from_meters(cls, value):
        return cls(value)

    @classmethod
    def from_feet(cls, value):
        return cls(value * cls.meter_per_feet)

    @property
    def in_feet(self):
        return self.in_meters / self.meter_per_feet

    def __eq__(self, other):
        if not isinstance(other, self.__class__):
            return False
        return self.in_meters == other.in_meters

Answer 3

First, do not repeat the conversion code in each object. Instead, have a UnitConverter helper class that does the calculations.

Second, the thing you want to store is a value and the corresponding unit. This more abstract concept can be reused for other measurements, e.g. length and weight.

Start be defining an enum for the supported units:

public enum UnitOfMeasurement {
    Invalid = 0,
    Meter = 1,
    Centimeter = 2,
    Foot = 3,
    Inch = 4,
    Kilogram = 5
}

Measurement class combines value and unit and can be used for any type of physical measurement:

public class Measurement : IEquatable<Measurement> {

    // two doubles are considered equal if their difference is smaller than epsilon
    private const epsilon = 0.000001; 

    public Measurement(double val, UnitOfMeasurement unit) {
        Value = val;
        Unit = unit;
    }

    public double Value { get; set; }
    public UnitOfMeasurement Unit { get; set; }

    // not sure if the standard Equals should consider two measurements the same if they have different units; 
    // maybe it would be better to introduce a separate EqualValueAfterConversion() method?
    public bool Equals(Measurement other) {
        if (ReferenceEquals(null, other)) return false;
        if (ReferenceEquals(this, other)) return true;
        try {
           var afterConversion = UnitConverter.Convert(other, this.Unit);
           return Math.Abs(Value - afterConversion.Value) < epsilon;
        } 
        catch (NotSupportedException) { 
           // units can not be converted
           return false;
        }
    }

    public override bool Equals(object obj) {
        if (ReferenceEquals(null, obj)) return false;
        if (ReferenceEquals(this, obj)) return true;
        if (obj.GetType() != this.GetType()) return false;
        return Equals((Measurement) obj);
    }

    // this does not produce the same Hash for two measurements that are considered equal, but have different units :(
    public override int GetHashCode() => Value.GetHashCode() ^ Unit.GetHashCode();
}

UnitConverter first checks whether the unit of one measurement can be converted into the requested unit; if it does, it performs the conversion, else it throws an exception.

public static class UnitConverter {

    private const double InchesPerMeter = 39.3701;

    public static Measurement Convert(Measurement measurement, UnitOfMeasurement targetUnit) {
        if (measurement.Unit == targetUnit) {
            return measurement;
        }

        switch (measurement.Unit) {
           case UnitOfMeasurement.Meter:
               switch (targetUnit) {
                   case UnitOfMeasurement.Inch:
                       return ConvertMetersToInches(measurement.Value);
                   case UnitOfMeasurement.Foot:
                   // ...

                   default: throw new NotSupportedException(
                       $"Source unit '{measurement.Unit}' can not be converted to target unit '{targetUnit}'.");

               }
               break;

           case UnitOfMeasurement.Inch:
           // ...

           default: throw new NotSupportedException(
               $"Source unit '{measurement.Unit}' can not be converted.");
        }
    }

    private static Measurement ConvertMetersToInches(double valueInMeters) {
        return new Measurement(valueInMeters * InchesPerMeter, UnitOfMeasurement.Inch);
    }
}

Answer 4

I would agree that you probably should use a canonical unit, but there is a point in not choosing a canonical unit but instead keeping them apart.

That way you can always retrieve the initial value in its initial form and not have to worry about getting a 189,0000001 cm length.

I can think of two reasonable ways to do that.

Internal dictionary

1. Have a set of units as an enum, say UnitEnum
2. Store the initial value along with the enum, say via a constructor public Height(UnitEnum, double)
3. Create a Dictionary<UnitEnum, double> statically that transform to a canonical unit.
4. Create a function double AsUnit(UnitEnum unit) to convert it to a specific. If currentUnit == unit than you just return the current value.

Object oriented madness

1. Create a base class Height that stores values in metres, and have a few overrideable functions/properties; such as string Unit {get;}, double AsMeters {get;}
2. Create subclasses HeightInch (and so on) that inherits from Height, and takes a Height as constructor and that converts the general Height to itself.
3. To convert a general Height to a specific unit, just create an object of that class (say HeightInch), an initialize it with the Height you got.

The upside of this is that you can add new units without ever touching any old code. I think it's overkill for this problem.

Answer 5

In addition to what has been discussed so far, your equality comparison code can be improved:

1. The code duplication between the two Equals overloads is unnecessary.
2. The identity comparison is redundant and probably constitutes a premature pessimisation: in typical code, this comparison is rarely expected to be true, thus lengthening rather than shortening the typical execution path.
3. In C# ≥7, you can replace ReferenceEquals(obj, null) with obj is null.
4. Given that your class represents a value, you should override == and != to be consistent with Equals (the IEquatable(T) documentation also mentions this).
5. Your floating point equality comparisons are broken, and cannot be fixed trivially. The simplest answer is probably to compare the values with some tolerance. Alternatively, implement the IComparable(T) interface and let the user deal with it.

On the other hand, well done on implementing GetHashCode.

The link above contains a detailed discussion of equality comparison in .NET. In your case, the relevant code would be:

public bool Equals(Height other)
{
    if (other is null) return false;
    if (GetType() != other.GetType()) return false; // (1)
    return ApproximatelyEqual(InMeters, other.InMeters);
}

private static bool ApproximatelyEqual(double a, double b)
{
    // See <https://stackoverflow.com/q/4915462/1968>
}

public override bool Equals(object obj) => Equals(obj as Height);

public static bool operator ==(Height lhs, Height rhs) => Equals(lhs, rhs);

public static bool operator !=(Height lhs, Height rhs) => ! (lhs == rhs);

public override int GetHashCode() => InMeters.GetHashCode();

Note that the line marked by (1) is only required if the class is supposed to be subclassed, and if you don’t want to consider objects of subclasses of Height equal to objects of class Height. You should probably remove it and mark your class sealed.

Answer 6

Most of the answers here are good, but I'd like to suggest a slightly different approach.

First and foremost, units of measurement are values, and I think they should be treated as such by representing them as value types (i.e. structs). There's a possibility that using a struct may also increase performance slightly in some situations, but the value semantics and immutability involved are far more important than that.

So first, define a series of structs for each unit you will be handling, like so: (Excuse the lack of newer features, I haven't used C# much since C# 4.0/4.5 was current version.)

public struct Metres : IEquatable<Metres>, IComparable<Metres>
{
    public static readonly string Suffix = "m";

    private double value;

    public Metres(double value)
    {
        this.value = value;
    }

    public override int GetHashCode()
    {
        return this.value.GetHashCode();
    }

    public override string ToString()
    {
        return this.value.ToString() + Suffix;
    }

    public override bool Equals(object obj)
    {
        if (obj == null)
            return false;

        if (obj is Metres)
            return this.Equals((Metres)obj);

        return false;
    }

    public bool Equals(Metres other)
    {
        return this.value == other.value;
    }

    public int CompareTo(Metres other)
    {
        if (this > other)
            return 1;

        if (this < other)
            return -1;

        return 0;
    }

    public static explicit operator Metres(double metres)
    {
        return new Metres(metres);
    }

    public static explicit operator double(Metres metres)
    {
        return metres.value;
    }

    public static bool operator ==(Metres left, Metres right)
    {
        return left.value == right.value;
    }

    public static bool operator !=(Metres left, Metres right)
    {
        return left.value != right.value;
    }

    public static bool operator >(Metres left, Metres right)
    {
        return left.value > right.value;
    }

    public static bool operator >=(Metres left, Metres right)
    {
        return left.value >= right.value;
    }

    public static bool operator <(Metres left, Metres right)
    {
        return left.value < right.value;
    }

    public static bool operator <=(Metres left, Metres right)
    {
        return left.value <= right.value;
    }
}

I suspect this would probably be a lot shorter using more modern features. Essentially it's mainly just a wrapper around a double, with a bit of custom behaviour. I implemented IEquatable and IComparable as standard and added appropriate operator overloads.

Before I go further, a few caveats:

Firstly, as another answer pointed out, doubles can be a bit difficult to test equality for, and there's other issues like infinity and NaN, so those operations aren't going to be perfect. If you need to be that precise though, I'd suggest creating a helper class to handle double comparisons and use that.

Secondly, ToString here is not culture sensitive. ToString will convert to a representation useful for a western culture, but not for other cultures, so if your program is going to be targetting other cultures you may need to rewrite ToString or even come up with a more robust solution, perhaps using a specialised LengthUnitFormatter.

Thirdly, I chose to make the conversion from Metres to double an explicit conversion to be safe. You might find that in your situation it's acceptable or even desired to make that particular conversion implicit. Conversely, the conversion from double to Metres should always be a strong explicit conversion.

And finallly, if you find that you need to be doing a lot of arithmetic with the units, you may wish to add some mathematical operators. If you do, you should make sure that they return the same kind of unit and only operate on either a unit and a double or a unit and the same unit. Personally I'd suggest making + and - only working with two values of the unit because adding an arbitrary number to a unit doesn't make much sense (i.e. unit + 5 doesn't make sense because the scale depends on the unit), whereas with * and / operating on an arbitrary number does make sense because it scales the value, (i.e. unit * 2 makes sense - it doubles the amount, regardless of unit).

---

So, you do that for all the units that you're going to need (they should all look very similar), and then you create a helper class to handle the conversions:

public static class LengthConversions
{
    private const double metreToCentimetreRatio = 1.0 / 100.0;

    public static Centimetres ToCentimetres(Metres metres)
    {
        return new Centimetres((double)metres * metreToCentimetreRatio);
    }

    private const double centimetreToMetreRatio = 100.0 / 1.0;

    public static Metres ToMetres(Centimetres centimetres)
    {
        return new Metres((double)centimetres * centimetreToMetreRatio);
    }
}

Thus you have a single canonical representation of how to convert the units.

After that you should represent your person's Height property with a canonical unit. I'd recommend using a metric unit rather than an imperial unit because metric units were designed with many useful mathematical properties in mind. Either metres or centimetres would be best for the human height range. (I'd lean more towards centimetres as it gives a nice balance of integer and fractional parts - not too big and not too small.)

Then whenever you have to convert the units you use the LengthConversions helper.

Answer 7

IMO Height is a too specific name for a measurement type. A better name would be Measurement because that will make it usable for more situations.

I would then make it a struct to adapt to the value types it will replace.

---

On the other hand I would consider this kind of type very carefully if it is really worth the trouble. If you have a large volume of code, you'll have to refactor it all to use the new type - code that supposedly is already working well.

Instead I would keep the code as is for now, and then implement the necessary extension methods to double (double.ToMetric() and double.ToImperial()) to be used as interface when (new) values "cross" from one system to the other.

Answer 8

Odd that you have public static Height FromCMeters(double heightInCms)
but no public static Height FromMeters(double heightInMeters)
and no public double InCMeters { get; private set; }

This conversion is not required

InMeters = (double) heightInCms / 100,

Userr

InMeters = heightInCms / 100d,

As said before you should define the constants

Consider just have one master unit and convert to from it

    public class Height : IEquatable<Height>
    {
        private Height()
        { }

        private static double FeetToInches = 12d;
        private static double FeetToMeters = 0.3048d;
        private static double InchesToMeters = FeetToMeters / FeetToInches;

        private double meters;
        public double Meters
        {
            get { return meters; }
            set { meters = value;
            }
        }

        public double Feet
        {
            get { return meters / FeetToMeters; }
            set { meters = value * FeetToMeters;  }
        }

        public double Inches
        {
            get { return meters / InchesToMeters; }
            set { meters = value * InchesToMeters; }
        }