Should this immutable struct be a mutable class?

Question

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
}

Answer 1

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.

Answer 2

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.

Answer 3

I agree that this should be an immutable type. But why this struct should implement a ICLoneable and IEquatable interface? It is a value type.

Answer 4

Personally, I feel that leaving this as an immutable struct is a very good thing. I would not recommend changing it to a mutable class.

Most of the time, in my experience, people wanting to avoid immutable structs are doing this from laziness. Immutable structs force you to recreate the struct will full parameters, but good constructor overloads can help tremendously here. (For an example, look at this Font constructor - even though it's a class, it implements a "clone everything but this variable" pattern that you can duplicate for your common fields that need changing.)

Creating mutable classes introduces other issues and overhead that I would avoid unless necessary.

Answer 5

Perhaps your co-worker could be satisfied by a set of methods to allow individual fields to be easily "changed" (resulting in a new instance with the the same values as the first instance except for the new field).

public PhoneNumber ApplyAreaCode(int areaCode)
{
  return new PhoneNumber(
    areaCode, 
    centralOfficeCode, 
    subscriberNumber, 
    extension);
}

Also, you could have a special case for an "undefined" phone number:

public static PhoneNumber Empty
{ get {return default(PhoneNumber); } }

public bool IsEmpty
{ get { return this.Equals(Empty); } }

The "Empty" property gives a more natural syntax than either "default(PhoneNumber) or new PhoneNumber()" and allows for the equivalent of a null check with either "foo == PhoneNumber.Empty" or foo.IsEmpty.

Also... In your TryParse don't you mean to

return result.value != 0;

Answer 6

Nullability can be easily handled via PhoneNumber?

Answer 7

Data holders which are going to be piecewise-alterable should be structures, rather than classes. While one can debate whether structures should be piecewise alterable, mutable classes make lousy data holders.

The problem is that every class object effectively contains two kinds of information:

1. The content of all of its fields
2. The whereabouts of all the references that exist to it

If a class object is immutable, it generally won't matter what references exist to it. When a data-holding class object is mutable, however, all references to it are effectively "attached" to each other; any mutation performed on one of them will effectively be performed on all.

If PhoneNumber were a mutable struct, one could change the fields of one storage location of type PhoneNumber without affecting any fields in any other storage location of that type. If one were to say var temp = Customers("Fred").MainPhoneNumber; temp.Extension = "x431"; Customers("Fred").MainPhoneNumber = temp; that would change Fred's extension without affecting anyone else's. By contrast, if PhoneNumber were a mutable class, the above code would set the extension for everyone whose MainPhoneNumber held a reference to the same object, but not affect the extension of anyone whose MainPhoneNumber held identical data but was not the same object. Icky.