Должна ли эта неизменяемая структура быть изменяемым классом? - PullRequest
9 голосов
/ 17 марта 2010

Я показал эту структуру другим программистам, и они почувствовали, что это должен быть изменяемый класс. Они чувствовали, что неудобно не иметь нулевые ссылки и возможность изменять объект по мере необходимости. Мне бы очень хотелось узнать, есть ли другие причины сделать этот изменчивый класс.

[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
}

Ответы [ 7 ]

21 голосов
/ 18 марта 2010

Программа, которая манипулирует номером телефона, является моделью процесса.

Поэтому сделайте вещи, которые являются неизменяемыми в процессе, неизменяемыми в коде. Сделайте вещи, которые изменяемы в процессе, изменяемыми в коде.

Например, процесс, вероятно, включает человека. У человека есть имя. Человек может изменить свое имя, сохраняя при этом свою идентичность. Поэтому имя объекта персонажа должно быть изменяемым.

У человека есть номер телефона. Человек может изменить свой номер телефона, сохраняя при этом свою личность. Поэтому номер телефона человека должен быть изменяемым.

Номер телефона имеет код города. Номер телефона НЕ МОЖЕТ изменить свой код города и сохранить свою личность; Вы изменили код города, теперь у вас есть другой номер телефона. Поэтому код города телефонного номера должен быть неизменным.

8 голосов
/ 17 марта 2010

Я думаю, что это хорошо, чтобы сохранить его как неизменную структуру - но я бы лично использовал отдельные переменные для каждого из логических полей, если вы не собираетесь иметь огромные их числа в памяти на время. Если вы придерживаетесь наиболее подходящего типа (например, ushort для 3-4 цифр), тогда это не должно быть , что дорого - и код будет чертовски понятнее.

2 голосов
/ 18 марта 2010

Возможно, ваш коллега мог бы быть удовлетворен набором методов, позволяющих легко "изменять" отдельные поля (в результате получается новый экземпляр с теми же значениями, что и первый экземпляр, за исключением нового поля).

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

Кроме того, у вас может быть специальный случай для «неопределенного» номера телефона:

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

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

Свойство «Пустой» дает более естественный синтаксис, чем «default (PhoneNumber) или новый PhoneNumber ()», и допускает эквивалент пустой проверки с помощью «foo == PhoneNumber.Empty» или foo.IsEmpty.

Также ... В вашем TryParse вы не имеете в виду

return result.value != 0;
2 голосов
/ 18 марта 2010

Лично я чувствую, что оставить это как неизменную структуру - очень хорошая вещь. Я бы не рекомендовал менять его на изменяемый класс.

По моему опыту, большую часть времени люди, желающие избежать неизменных структур, делают это из-за лени. Неизменяемые структуры вынуждают вас воссоздавать структуру с полными параметрами, но хорошие перегрузки конструктора могут здесь сильно помочь. (Например, посмотрите на этот конструктор шрифтов - хотя он и является классом, он реализует шаблон «клонировать все, кроме этой переменной», который вы можете дублировать для общих полей, требующих изменения.)

Создание изменяемых классов приводит к другим проблемам и накладным расходам, которых я бы избегал, если бы не было необходимости.

2 голосов
/ 17 марта 2010

Я согласен, что это должен быть неизменный тип. Но почему эта структура должна реализовывать интерфейс ICLoneable и IEquatable? Это тип значения.

0 голосов
/ 09 августа 2012

Держатели данных, которые будут кусочно-изменяемыми, должны быть структурами, а не классами. В то время как можно спорить, должны ли структуры быть кусочно изменяемыми, изменяемые классы делают паршивых держателей данных .

Проблема в том, что каждый объект класса эффективно содержит два вида информации:

  1. Содержимое всех его полей
  2. местонахождение всех ссылок, которые существуют на него

Если объект класса является неизменным, обычно не имеет значения, какие ссылки на него существуют. Однако, когда объект класса, содержащий данные, является изменчивым, все ссылки на него фактически «привязаны» друг к другу; любая мутация, выполненная на одном из них, будет эффективно выполняться на всех.

Если бы PhoneNumber была изменяемой структурой, можно было бы изменить поля одного хранилища типа PhoneNumber, не затрагивая какие-либо поля в любом другом хранилище этого типа. Если бы вы сказали var temp = Customers("Fred").MainPhoneNumber; temp.Extension = "x431"; Customers("Fred").MainPhoneNumber = temp;, это изменило бы расширение Фреда, не затрагивая чье-либо еще. Напротив, если бы PhoneNumber был изменяемым классом, вышеприведенный код установил бы расширение для каждого, чья MainPhoneNumber содержала ссылку на тот же объект, но не влияло бы на расширение любого, чей MainPhoneNumber содержал идентичные данные, но не был тот же объект. Icky.

0 голосов
/ 18 марта 2010

Обнуляемость может быть легко обработана через PhoneNumber?

...