Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should this immutable struct be a mutable class?

I showed this struct to a fellow programmer and they felt that it should be a mutable class. They felt it is inconvenient not to have null references and the ability to alter the object as required. I would really like to know if there are any other reasons to make this a mutable class.

[Serializable]
public struct PhoneNumber : IEquatable<PhoneNumber>
{
    private const int AreaCodeShift = 54;
    private const int CentralOfficeCodeShift = 44;
    private const int SubscriberNumberShift = 30;
    private const int CentralOfficeCodeMask = 0x000003FF;
    private const int SubscriberNumberMask = 0x00003FFF;
    private const int ExtensionMask = 0x3FFFFFFF;


    private readonly ulong value;


    public int AreaCode
    {
        get { return UnmaskAreaCode(value); }
    }

    public int CentralOfficeCode
    {
        get { return UnmaskCentralOfficeCode(value); }
    }

    public int SubscriberNumber
    {
        get { return UnmaskSubscriberNumber(value); }
    }

    public int Extension
    {
        get { return UnmaskExtension(value); }
    }


    public PhoneNumber(ulong value)
        : this(UnmaskAreaCode(value), UnmaskCentralOfficeCode(value), UnmaskSubscriberNumber(value), UnmaskExtension(value), true)
    {

    }

    public PhoneNumber(int areaCode, int centralOfficeCode, int subscriberNumber)
        : this(areaCode, centralOfficeCode, subscriberNumber, 0, true)
    {

    }

    public PhoneNumber(int areaCode, int centralOfficeCode, int subscriberNumber, int extension)
        : this(areaCode, centralOfficeCode, subscriberNumber, extension, true)
    {

    }

    private PhoneNumber(int areaCode, int centralOfficeCode, int subscriberNumber, int extension, bool throwException)
    {
        value = 0;

        if (areaCode < 200 || areaCode > 989)
        {
            if (!throwException) return;
            throw new ArgumentOutOfRangeException("areaCode", areaCode, @"The area code portion must fall between 200 and 989.");
        }
        else if (centralOfficeCode < 200 || centralOfficeCode > 999)
        {
            if (!throwException) return;
            throw new ArgumentOutOfRangeException("centralOfficeCode", centralOfficeCode, @"The central office code portion must fall between 200 and 999.");
        }
        else if (subscriberNumber < 0 || subscriberNumber > 9999)
        {
            if (!throwException) return;
            throw new ArgumentOutOfRangeException("subscriberNumber", subscriberNumber, @"The subscriber number portion must fall between 0 and 9999.");
        }
        else if (extension < 0 || extension > 1073741824)
        {
            if (!throwException) return;
            throw new ArgumentOutOfRangeException("extension", extension, @"The extension portion must fall between 0 and 1073741824.");
        }
        else if (areaCode.ToString()[1] == '9')
        {
            if (!throwException) return;
            throw new ArgumentOutOfRangeException("areaCode", areaCode, @"The second digit of the area code cannot be greater than 8.");
        }
        else
        {
            value |= ((ulong)(uint)areaCode << AreaCodeShift);
            value |= ((ulong)(uint)centralOfficeCode << CentralOfficeCodeShift);
            value |= ((ulong)(uint)subscriberNumber << SubscriberNumberShift);
            value |= ((ulong)(uint)extension);
        }
    }


    public override bool Equals(object obj)
    {
        return obj != null && obj.GetType() == typeof(PhoneNumber) && Equals((PhoneNumber)obj);
    }

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

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

    public override string ToString()
    {
        return ToString(PhoneNumberFormat.Separated);
    }

    public string ToString(PhoneNumberFormat format)
    {
        switch (format)
        {
            case PhoneNumberFormat.Plain:
                return string.Format(@"{0:D3}{1:D3}{2:D4}{3:#}", AreaCode, CentralOfficeCode, SubscriberNumber, Extension).Trim();
            case PhoneNumberFormat.Separated:
                return string.Format(@"{0:D3}-{1:D3}-{2:D4} {3:#}", AreaCode, CentralOfficeCode, SubscriberNumber, Extension).Trim();
            default:
                throw new ArgumentOutOfRangeException("format");
        }
    }

    public ulong ToUInt64()
    {
        return value;
    }


    public static PhoneNumber Parse(string value)
    {
        var result = default(PhoneNumber);
        if (!TryParse(value, out result))
        {
            throw new FormatException(string.Format(@"The string ""{0}"" could not be parsed as a phone number.", value));
        }
        return result;
    }

    public static bool TryParse(string value, out PhoneNumber result)
    {
        result = default(PhoneNumber);

        if (string.IsNullOrEmpty(value))
        {
            return false;
        }

        var index = 0;
        var numericPieces = new char[value.Length];

        foreach (var c in value)
        {
            if (char.IsNumber(c))
            {
                numericPieces[index++] = c;
            }
        }

        if (index < 9)
        {
            return false;
        }

        var numericString = new string(numericPieces);
        var areaCode = int.Parse(numericString.Substring(0, 3));
        var centralOfficeCode = int.Parse(numericString.Substring(3, 3));
        var subscriberNumber = int.Parse(numericString.Substring(6, 4));
        var extension = 0;

        if (numericString.Length > 10)
        {
            extension = int.Parse(numericString.Substring(10));
        }

        result = new PhoneNumber(
            areaCode,
            centralOfficeCode,
            subscriberNumber,
            extension,
            false
        );

        return result.value != 0;
    }

    public static bool operator ==(PhoneNumber left, PhoneNumber right)
    {
        return left.Equals(right);
    }

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

    private static int UnmaskAreaCode(ulong value)
    {
        return (int)(value >> AreaCodeShift);
    }

    private static int UnmaskCentralOfficeCode(ulong value)
    {
        return (int)((value >> CentralOfficeCodeShift) & CentralOfficeCodeMask);
    }

    private static int UnmaskSubscriberNumber(ulong value)
    {
        return (int)((value >> SubscriberNumberShift) & SubscriberNumberMask);
    }

    private static int UnmaskExtension(ulong value)
    {
        return (int)(value & ExtensionMask);
    }
}

public enum PhoneNumberFormat
{
    Plain,
    Separated
}
like image 335
ChaosPandion Avatar asked Mar 17 '10 20:03

ChaosPandion


2 Answers

A program that manipulates a phone number is a model of a process.

Therefore, make things which are immutable in the process immutable in the code. Make things which are mutable in the process mutable in the code.

For example, a process probably includes a person. A person has a name. A person can change their name while retaining their identity. Therefore, the name of a person object should be mutable.

A person has a phone number. A person can change their phone number while retaining their identity. Therefore, the phone number of a person should be mutable.

A phone number has an area code. A phone number CANNOT change its area code and retain its identity; you change the area code, you now have a different phone number. Therefore the area code of a phone number should be immutable.

like image 100
Eric Lippert Avatar answered Oct 18 '22 09:10

Eric Lippert


I think it's fine to keep it as an immutable struct - but I would personally just use separate variables for each of the logical fields unless you're going to have huge numbers of these in memory at a time. If you stick to the most appropriate type (e.g. ushort for 3-4 digits) then it shouldn't be that expensive - and the code will be a heck of a lot clearer.

like image 45
Jon Skeet Avatar answered Oct 18 '22 08:10

Jon Skeet