I am creating a class to be the end all defacto class for handling unit conversions. This is particularly difficult in my company's industry where imperial architectural strings are used as units. I wanted this class to be perfect in just about every way as it is the class that we will be giving our summer interns as the best example of how to write a class. By posting it here, I wanted to make sure that it is the best example! The references to the DimensionConverter class is to a private class built just for this purpose. (I would like it reviewed as well... but I don't know how to post two classes appropriately here on Code Review.)What else could I possibly improve about the way this class is written in the context of readability, ease of maintenance, and logical design?namespace TypeLibrary{    /// <summary>    /// Class used for storing dimensions that may need to be accessed in a different measurement system    /// Will accept anything as input    /// <example>    /// decimal inches into AutoCAD notation    ///     /// double inches = 14.1875    /// Dimension dm = new Dimension( inches, DimensionTypes.Inch);    ///     /// Print(dm.Architectural)    ///     /// ========Output==========    /// 1'2 3/16    ///     /// </example>    /// </summary>    public class Dimension    {        #region internal variables        //internal Dimension type is set to milimeter to cause the least amount of rounding error when performing calculations.        const DimensionTypes internalUnitType = DimensionTypes.Millimeter;        //dimension value        private double internalDecimalUnit = 0.0;        #endregion        #region Constructors        /// <summary>        /// Accepts any string value for input.        /// </summary>        public Dimension(string passedArchitecturalString)        {             storeArchitecturalStringAsInternalUnit(passedArchitecturalString);        }        /// <summary>        /// Accepts any type of value for input. This allows for less complexity in building applications        /// </summary>        public Dimension(double passedInput, DimensionTypes passedDimensionType)        {            internalDecimalUnit = DimensionConverter.ConvertDimension(passedDimensionType, passedInput, internalUnitType);        }        /// <summary>        /// Copy Constructor        /// </summary>        public Dimension(Dimension passedDimension)        {            this.internalDecimalUnit = passedDimension.internalDecimalUnit;        }        #endregion        #region Properties        public double Sixteenths        {            get { return retrieveAsExternalUnit(DimensionTypes.Sixteenth); }            set { storeAsInternalUnit(DimensionTypes.Sixteenth, value); }        }        public double ThirtySeconds        {            get { return retrieveAsExternalUnit(DimensionTypes.ThirtySecond); }            set { storeAsInternalUnit(DimensionTypes.ThirtySecond, value); }        }        public double Inches        {            get { return retrieveAsExternalUnit(DimensionTypes.Inch); }            set { storeAsInternalUnit(DimensionTypes.Inch, value); }        }        public double Feet        {            get { return retrieveAsExternalUnit(DimensionTypes.Foot); }            set { storeAsInternalUnit(DimensionTypes.Foot, value); }        }        public double Yards        {            get { return retrieveAsExternalUnit(DimensionTypes.Yard); }            set { storeAsInternalUnit(DimensionTypes.Yard, value); }        }        public double Miles        {            get { return retrieveAsExternalUnit(DimensionTypes.Mile); }            set { storeAsInternalUnit(DimensionTypes.Mile, value); }        }        public double Millimeters        {            get { return retrieveAsExternalUnit(DimensionTypes.Millimeter); }            set { storeAsInternalUnit(DimensionTypes.Millimeter, value); }        }        public double Centimeters        {            get { return retrieveAsExternalUnit(DimensionTypes.Centimeter); }            set { storeAsInternalUnit(DimensionTypes.Centimeter, value); }        }        public double Meters        {            get { return retrieveAsExternalUnit(DimensionTypes.Meter); }            set { storeAsInternalUnit(DimensionTypes.Meter, value); }        }        public double Kilometers        {            get { return retrieveAsExternalUnit(DimensionTypes.Kilometer); }            set { storeAsInternalUnit(DimensionTypes.Kilometer, value); }        }        /// <summary>        /// Returns the dimension as a string in AutoCAD notation with sixteenths of an inch percision        /// </summary>        public string Architectural        {            get { return retrieveInternalUnitAsArchitecturalString(); }            set {storeArchitecturalStringAsInternalUnit(value); }        }        #endregion        #region helper methods        private void storeAsInternalUnit(DimensionTypes fromDimensionType, double passedValue)        {            internalDecimalUnit = DimensionConverter.ConvertDimension( DimensionTypes.Kilometer, passedValue, internalUnitType);        }        private void storeArchitecturalStringAsInternalUnit(string passedArchitecturalString)        {            internalDecimalUnit = DimensionConverter.ConvertArchitecturalStringToDecimalDimension(internalUnitType, passedArchitecturalString);        }        private double retrieveAsExternalUnit(DimensionTypes toDimensionType)        {            return DimensionConverter.ConvertDimension(internalUnitType, internalDecimalUnit, toDimensionType);        }        private string retrieveInternalUnitAsArchitecturalString()        {            return DimensionConverter.ConvertDecimalDimensionToArchitecturalString(internalUnitType, internalDecimalUnit);        }        #endregion        #region Overloaded Operators        /* You may notice that we do not overload the increment and decrement operators.         * This is because the user of this library         * does not know what is being internally stored. We could have allowed a parameter         * into a custom operater and method of our own, but that would have added unnecessary         * complexity.         */        public static Dimension operator +(Dimension d1, Dimension d2)        {            //add the two dimensions together            //return a new dimension with the new value            return new Dimension((d1.internalDecimalUnit + d2.internalDecimalUnit), internalUnitType);        }        public static Dimension operator -(Dimension d1, Dimension d2)        {            //subtract the two dimensions            //return a new dimension with the new value            return new Dimension((d1.internalDecimalUnit - d2.internalDecimalUnit), internalUnitType);        }        public static Dimension operator *(Dimension d1, Dimension d2)        {            //multiply the two dimensions            //return a new dimension with the new value            return new Dimension((d1.internalDecimalUnit * d2.internalDecimalUnit), internalUnitType);        }        public static Dimension operator /(Dimension d1, Dimension d2)        {            //divide the two dimensions            //return a new dimension with the new value            return new Dimension((d1.internalDecimalUnit / d2.internalDecimalUnit), internalUnitType);        }        public static Dimension operator %(Dimension d1, Dimension d2)        {            //Modulous the two dimensions            //return a new dimension with the new value            return new Dimension((d1.internalDecimalUnit % d2.internalDecimalUnit), internalUnitType);        }        public static bool operator ==(Dimension d1, Dimension d2)        {            //compare the two dimensions for equality            //return a bool based on their relative values            if (d1.internalDecimalUnit == d2.internalDecimalUnit)             {                return true;            }            else            {                return false;            }        }        public static bool operator !=(Dimension d1, Dimension d2)        {            //compare the two dimensions for equality            //return a bool based on their relative values            if (d1.internalDecimalUnit != d2.internalDecimalUnit)            {                return true;            }            else            {                return false;            }        }        public static bool operator >(Dimension d1, Dimension d2)        {            //compare the two dimensions together            //return a bool based on their relative values            if (d1.internalDecimalUnit > d2.internalDecimalUnit)            {                return true;            }            else            {                return false;            }        }        public static bool operator <(Dimension d1, Dimension d2)        {            //compare the two dimensions together            //return a bool based on their relative values            if (d1.internalDecimalUnit < d2.internalDecimalUnit)            {                return true;            }            else            {                return false;            }        }        /// <summary>        /// This override determines how this object is inserted into hashtables.        /// </summary>        /// <returns>same hashcode as any double would</returns>        public override int GetHashCode()        {            return internalDecimalUnit.GetHashCode();        }        public override string ToString()        {            throw new NotImplementedException(The Dimension class does not know what type of unit it contains, (Because it should be thought of containing all unit types) Call dimension.[unit].Tostring() instead);        }        #endregion    }}The second class that does the real worknamespace TypeLibrary{    /// <summary>     /// Contains all dimension conversion tools    /// </summary>    internal static class DimensionConverter    {        #region private functions        /// <summary>        /// Converts any possible type of Architectual String into Millimeters        /// <remarks>Throws FormatException on bad input</remarks>        /// </summary>        /// <param name=arch>the input string</param>        /// <returns>decimal Millimeters</returns>        private static Double ConvertArchitectualStringtoMillimeters(String arch)        {            // for details on where this solution came from, check here: http://stackoverflow.com/questions/22794466/parsing-all-possible-types-of-varying-architectural-dimension-input            // answer by Trygve Flathen: http://stackoverflow.com/users/2795177/trygve-flathen            String expr = ^\\s*(?<minus>-)?\\s*(((?<feet>\\d+)(?<inch>\\d{2})(?<sixt>\\d{2}))|((?<feet>[\\d.]+)')?[\\s-]*((?<inch>\\d+)?[\\s-]*((?<numer>\\d+)/(?<denom>\\d+))?\)?)\\s*$;            Match m = new Regex(expr).Match(arch);            if (!m.Success || arch.Trim() ==  || arch.Trim() == \)            {                FormatException exception = new FormatException(Input was not a valid architectural string);                ExceptionHandler.ProcessException(exception);                throw exception;            }            Int32 sign = m.Groups[minus].Success ? -1 : 1;            Double feet = m.Groups[feet].Success ? Convert.ToDouble(m.Groups[feet].Value) : 0;            Int32 inch = m.Groups[inch].Success ? Convert.ToInt32(m.Groups[inch].Value) : 0;            Int32 sixt = m.Groups[sixt].Success ? Convert.ToInt32(m.Groups[sixt].Value) : 0;            Int32 numer = m.Groups[numer].Success ? Convert.ToInt32(m.Groups[numer].Value) : 0;            Int32 denom = m.Groups[denom].Success ? Convert.ToInt32(m.Groups[denom].Value) : 1;            double result = sign * (feet * 12 + inch + sixt / 16.0 + numer / Convert.ToDouble(denom));            return ConvertDimension(DimensionType.Millimeter, result, DimensionType.Inch);        }        /// <summary>        /// Returns a string formatted in a standard AutoCAD format        /// </summary>        /// <param name=num> the number being converted into a string</param>        /// <returns></returns>        private static string ConvertMillimetersToArchitecturalString(double num, int precision = 16)        {            //Convert into inches before proceeding            num = ConvertDimension(DimensionType.Inch, num, DimensionType.Millimeter);            string functionReturnValue = ;            int feet = 0;            int inches = 0;            string FeetString = ;            string InchesString = ;            int numerator = 0;            string sign = ;            string fractionString = ;            //detect need for sign            if (num < 0)            {                sign = -;                num = num * -1;            }            feet = Convert.ToInt32(num / 12);            num = num - (feet * 12);            inches = Convert.ToInt32(num);            num = num - inches;            numerator = Convert.ToInt32(num * precision);            if (numerator >= 32)            {                numerator = numerator - precision;                inches = inches + 1;            }            fractionString = numerator + / + precision.ToString() + \;            if (feet == 0)            {                FeetString = ;            }            else            {                FeetString = Convert.ToString(feet);            }            if (inches == 0)            {                InchesString = ;            }            else            {                InchesString = Convert.ToString(inches);            }            if (string.IsNullOrEmpty(sign) & string.IsNullOrEmpty(FeetString))            {                functionReturnValue = (InchesString +   + fractionString).Trim();            }            else            {                functionReturnValue = (sign + FeetString + '  + InchesString +   + fractionString).Trim();            }            return functionReturnValue;        }        #endregion        #region public functions        /// <summary>        /// Converts any dimension into another        /// </summary>        /// <param name=typeConvertingTo>input unit type</param>        /// <param name=typeConvertingFrom>desired output unit type</param>        /// <returns>converted dimension</returns>        public static double ConvertDimension(DimensionType typeConvertingFrom, double passedValue, DimensionType typeConvertingTo)        {            double returnDouble = 0.0;            double internalDecimalMillimeters = 0.0;            //first convert value passedValue to inches            switch (typeConvertingFrom)            {                case DimensionType.ThirtySecond:                    internalDecimalMillimeters = (passedValue / 32) * 0.0393701;                    break;                case DimensionType.Sixteenth:                    internalDecimalMillimeters = (passedValue / 16) * 0.0393701;                    break;                case DimensionType.Inch:                    internalDecimalMillimeters = passedValue * 25.4;                    break;                case DimensionType.Foot:                    internalDecimalMillimeters = passedValue * 304.8;                    break;                case DimensionType.Yard:                    internalDecimalMillimeters = passedValue * 914.4;                    break;                case DimensionType.Mile:                    internalDecimalMillimeters = passedValue * 1609344;                    break;                case DimensionType.Millimeter:                    internalDecimalMillimeters = passedValue;                    break;                case DimensionType.Centimeter:                    internalDecimalMillimeters = passedValue * 10;                    break;                case DimensionType.Meter:                    internalDecimalMillimeters = passedValue * 1000;                    break;                case DimensionType.Kilometer:                    internalDecimalMillimeters = passedValue * 1000000;                    break;            }            //Now convert the value from inches to the desired output            switch (typeConvertingTo)            {                case DimensionType.ThirtySecond:                    returnDouble = (internalDecimalMillimeters / 0.0393701) * 32;                    break;                case DimensionType.Sixteenth:                    returnDouble = (internalDecimalMillimeters / 0.0393701) * 16;                    break;                case DimensionType.Inch:                    returnDouble = internalDecimalMillimeters / 25.4;                    break;                case DimensionType.Foot:                    returnDouble = internalDecimalMillimeters / 304.8;                    break;                case DimensionType.Yard:                    returnDouble = internalDecimalMillimeters / 914.4;                    break;                case DimensionType.Mile:                    returnDouble = internalDecimalMillimeters / 1609344;                    break;                case DimensionType.Millimeter:                    returnDouble = internalDecimalMillimeters;                    break;                case DimensionType.Centimeter:                    returnDouble = internalDecimalMillimeters / 10;                    break;                case DimensionType.Meter:                    returnDouble = internalDecimalMillimeters / 1000;                    break;                case DimensionType.Kilometer:                    returnDouble = internalDecimalMillimeters / 1000000;                    break;            }            return returnDouble;        }        /// <summary>        /// Converts Architectural strings into a decimal unit        /// </summary>        public static double ConvertArchitecturalStringToDecimalDimension(DimensionType typeConvertingTo, string passedValue)        {            double internalDecimalMillimeters = 0.0;            //first convert value passedValue to millimeters            internalDecimalMillimeters = ConvertArchitectualStringtoMillimeters(passedValue);            //Now convert the value from millimeters to the desired output            double result = ConvertDimension(DimensionType.Millimeter, internalDecimalMillimeters, typeConvertingTo);            return result;        }        /// <summary>        /// Converts any dimension into an architectural string representation        /// </summary>        /// <param name=typeConvertingFrom>desired output unit type</param>        /// <returns>converted dimension</returns>        public static string ConvertDecimalDimensionToArchitecturalString(DimensionType typeConvertingFrom, double passedValue)        {            double internalDecimalMillimeters = 0.0;            //first convert value passedValue to millimeters            ConvertDimension(DimensionType.Millimeter, passedValue, typeConvertingFrom);            //Now convert the value from inches to the desired output            return ConvertMillimetersToArchitecturalString(internalDecimalMillimeters);        }        #endregion    }}EDITI have made some of the suggested changes from below. And I have written my UnitTests and come up with a few more questions. Some people have suggested that helper classes like my DimensionConverter class are evil. should I combine the utility class with the dimension class and if so how?Also what improvements should I make to my tests if I am assuming everything in DimensionConverter will be private to only the Dimension class?UnitTests for the Dimension Class:[TestClass]public class DimensionTests{    /// <summary>    /// Tests the architectural string constructor and the regular dimension constructor    /// </summary>    [TestMethod]    public void Dimensions_Constructors()    {        // arrange & act        //numeric value constructor        Dimension inchDimension = new Dimension(DimensionType.Inch, 14.1875);        //architectural string constructor        Dimension architecturalDimension = new Dimension(1' 2 3/16\);        //copy constructor        Dimension copiedDimension = new Dimension(architecturalDimension);        // assert        inchDimension.Millimeters.Should().Be(architecturalDimension.Millimeters);        copiedDimension.ShouldBeEquivalentTo(architecturalDimension);    }    /// <summary>    /// Tests mathmatical operators we will test the properties at the same time.    /// </summary>    [TestMethod]    public void Dimensions_Math_Operators()    {        // arrange        Dimension inchDimension = new Dimension(DimensionType.Inch, 14.1875);        Dimension architecturalDimension = new Dimension(1'2 3/16\);        // act        Dimension subtractionDimension = inchDimension - architecturalDimension;        Dimension additionDimension = inchDimension + architecturalDimension;        // assert        subtractionDimension.Feet.Should().BeApproximately(0, .00000001, Doubles math should get us at least this close);        additionDimension.Millimeters.Should().BeApproximately(720.725, .00000001, Doubles math should get us at least this close);        additionDimension.Architectural.ShouldBeEquivalentTo(2'4 6/16\);    }    /// <summary>    /// Tests Architectural string inputs.    /// </summary>    [TestMethod]    public void Dimensions_Architectural_Constructor()    {        // arrange        Dimension dimension1 = new Dimension(1'2 3/16\);        Dimension dimension2 = new Dimension(1');        Dimension dimension3 = new Dimension(1'2\);        Dimension dimension4 = new Dimension(2 3/16\);        Dimension dimension5 = new Dimension(1'2-3/16\);        Dimension dimension6 = new Dimension(3/16\);        Dimension dimension7 = new Dimension(121103);        Dimension dimension8 = new Dimension(-1'2\);        // assert        dimension1.Architectural.ShouldBeEquivalentTo(1'2 3/16\);        dimension2.Architectural.ShouldBeEquivalentTo(1');        dimension3.Architectural.ShouldBeEquivalentTo(1'2\);        dimension4.Architectural.ShouldBeEquivalentTo(2 3/16\);        dimension5.Architectural.ShouldBeEquivalentTo(1'2 3/16\);        dimension6.Architectural.ShouldBeEquivalentTo(3/16\);        dimension7.Architectural.ShouldBeEquivalentTo(12'11 3/16\);        dimension8.Architectural.ShouldBeEquivalentTo(-1'2\);    }    /// <summary>    /// Tests all equality operators    /// </summary>    [TestMethod]    public void Dimensions_Equality_Operators()    {        // arrange        Dimension biggerDimension = new Dimension(DimensionType.Inch, 14.1875);        Dimension smallerDimension = new Dimension(1' 2 1/16\);        Dimension equivalentbiggerDimension = new Dimension(DimensionType.Millimeter, 360.3625);        // assert        (smallerDimension < biggerDimension).Should().Be(true);        (biggerDimension < smallerDimension).Should().Be(false);        (biggerDimension > smallerDimension).Should().Be(true);        (smallerDimension > biggerDimension).Should().Be(false);        (equivalentbiggerDimension == biggerDimension).Should().Be(true);        (equivalentbiggerDimension == smallerDimension).Should().Be(false);        (equivalentbiggerDimension != smallerDimension).Should().Be(true);        (equivalentbiggerDimension != biggerDimension).Should().Be(false);    }    /// <summary>    /// Tests GetHashCodeOperation    /// </summary>    [TestMethod]    public void Dimensions_GetHashCode()    {        // arrange        Dimension dimension = new Dimension(DimensionType.Millimeter, 14.1875);        double number = 14.1875;        // act        int dimensionHashCode = dimension.GetHashCode();        int hashCode = number.GetHashCode();        // assert        hashCode.ShouldBeEquivalentTo(dimensionHashCode);    }    /// <summary>    /// Tests toString failure    /// </summary>    [TestMethod]    [ExpectedException(typeof(NotImplementedException))]    public void Dimensions_ToString()    {        // arrange        Dimension dimension = new Dimension(DimensionType.Millimeter, 14.1875);        // act        dimension.ToString();    }    /// <summary>    /// Tests CompareTo implementation    /// </summary>    [TestMethod]    public void Dimensions_CompareTo()    {        // arrange        Dimension smallDimension = new Dimension(DimensionType.Millimeter, 1);        Dimension mediumDimension = new Dimension(DimensionType.Foot, 1);        Dimension largeDimension = new Dimension(DimensionType.Kilometer, 1);        List<Dimension> dimensions = new List<Dimension>();        dimensions.Add(smallDimension);        dimensions.Add(largeDimension);        dimensions.Add(mediumDimension);        // act        dimensions.Sort();        // assert        dimensions[0].ShouldBeEquivalentTo(smallDimension);        dimensions[1].ShouldBeEquivalentTo(mediumDimension);        dimensions[2].ShouldBeEquivalentTo(largeDimension);    }}
Class for handling unit conversions
c#;strings;converting
VerbosityVerbosity can be a good thing in code. The method name explains what the method does. Too much verbosity, though, can make your code tiring to read, as well as less memorable. For example - passedArchitecturalString - why call it passed? we know it is passed, it is an argument. Same for String - we know it is a string. Simply calling architectural would have conveyed the same information, and be a lot more memorable. internalDecimalUnit - again, we know it is internal, and it is actually not Decimal (it is double), I believe value would have been enough here, perhaps intrinsicValue if you want to be explicit.CommentaryComments have had their day when they were considered crucial to code readability. Nowadays, though, they have lost their shine a bit, and sometimes considered as a code smell. The main problem with comments, is that they tend to be forgotten as the code progresses, and often start making no sense, as they are not synchronized with the code. Another problem with them is that most programmers skip comments, and never actually read them.Some of your comments are good (like for internalUnitType), but some are absolutely redundant (like for the copy constructor, or for the architectural constructor), and some just make the code harder to read rather than easier to read like the comments inside your operator overloadings.Code consistencyIt is a matter of style whether you want to address your internal members with the this. prefix, but once you make the choice, be consistent.When helpers don't helpYour helper methods are one-liner delegations. This seems a bit redundant, since they don't reduce code complexity, nor enhance readability. They may even introduce bugs (let's see if you can find the bug in storeAsInternalUnit) It always converts to KilometersAlso, some helpers have only a single use, which makes them even more a waste of space. I would think that, for example, the following constructor code is more readable than yours, and saves you a method:public Dimension(string architectural){     Architectural = architectural;}The ImmutablesYou decided not to override Equals, which is fine, but you did override GetHashCode. GetHashCode is rarely used in user code, but mainly in containers which use hashing to store/retrieve elements. Overriding this method might break your code in these cases, since your Dimension is not immutable, and changing one's value while it is in a container, will make it 'disappear' from any such container he might be in.Either make your class immutable, or delete this method. And if you decide to keep it, consider overriding Equals as well, they look so much better together...String it!You opted to not implement ToString in your code. This is problematic, since in many places in your user code, he may want to use traces or debug logs, and your decision to throw an error there will unnecessarily break their code. You might expect them to call dimension.[unit].ToString(), but what if they log a List of Dimension? I notice that you chose not to put the DimensionConverter code in this post, which is a shame, because I can guess all the juicy parts are there, and you let us glimpse only on code with no complexity since it only delegates to that mysterious class :-(...UpdateBefore I'll tackle the DimensionConverter, there are a couple of things I forgot to mention, which are very important, especially as you aim to have a perfect code:Naming ConventionsC# naming conventions state that const members as well as all methods should be PascalCase. This is especially important in the case of you elusive const DimensionTypes internalUnitType. Consider the following two lines:internalDecimalUnit = DimensionConverter.ConvertDimension(passedDimensionType,                       passedInput, internalUnitType);internalDecimalUnit = DimensionConverter.ConvertDimension(passedDimensionType,                       passedInput, InternalUnitType);Even SO's syntax highlighter sees the difference, which makes the intention of your constant a whole lot more apparent.Use DecimalWhen used for human units (base 10), it is always advisable to use decimal instead of double. Doubles are notorious in their quirks when they are involved in arithmetic.Unit testsNo self-respecting perfect code lacks a good fool-proof suite of unit tests! They make your code more stable, more readable and more maintainable. Educate your interns today on the importance of unit tests!String it to meYou asked me what should ToString() return by default?I think that on the most basic level, choose the format you feel is most appropriate in millimeters, or as architectural - the one most convenient to your users (remember to indicate which unit you are using in the resulting string!).If you feel like being a little more elaborate, you can provide a global flag, which the user can set, which tells the ToString() method which is the default unit to render.