这个不可变的结构体应该改成可变的类吗?

9

我向另外一位程序员展示了这个结构体,他们认为它应该是可变类。他们认为没有空引用和根据需要更改对象是不方便的。我真的很想知道是否有其他原因来使这个成为可变类。

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

1
除非你试图操作当地电话公司的交换机,否则这看起来对我来说是一个过度工程化的字符串。我不知道,我无法理解为什么它必须如此复杂。这本身就是一个问题。 - Hans Passant
@nobugz - 是我把所有东西都压缩成ulong的事实让它变得复杂吗? - ChaosPandion
“areaCode.ToString()[1] - 48 > 8” 这段代码从哪里来的?“areaCode.ToString()[1] == '9'” 显然好理解得多。别忘了 “areaCode % 100 == 11”。顺便说一下,你应该知道37X和96X也是保留区号。 - Gabe
@gabe - areaCode.ToString()[1] == '9'确实使代码更清晰。 - ChaosPandion
7个回答

21

操纵电话号码的程序是一个过程的模型。

因此,在代码中使过程中不可变的事物也不可变,使过程中可变的事物也可变。

例如,一个过程可能包含一个人。人有姓名。人可以更改自己的姓名而保留其身份。因此,人对象的名称应该是可变的。

人有电话号码。人可以更改其电话号码而保留其身份。因此,人的电话号码应该是可变的。

电话号码有区号。电话号码不能更改其区号并保留其身份;如果您更改了区号,则现在有了不同的电话号码。因此,电话号码的区号应该是不可变的。


那么你的意思是在Person上有一个可变字段,而电话号码是不可变的,这是错误的方向吗? - ChaosPandion
1
让我再试一次。字符串是不可变的。要将“Anderson”变成“Andreeson”,您不会修改“Anderson”;您完全丢弃它并替换整个内容。名称字段是可变的;包含任何给定名称的字符串则不是。电话号码也是如此。电话号码是不可变的;要将519-555-1212变为206-555-1212,您不会改变第一个号码,而是将其丢弃并重新开始。但是,人员的电话号码字段是可变的。 - Eric Lippert
明白了,我们现在意见一致。 - ChaosPandion
既然你在这里,也许你可以就“结构体”的设计发表一下你的意见? - ChaosPandion
你说电话号码不能更改其区号并保留其身份。这取决于人们如何解释电话号码的身份。这里周围很多人和企业会说他们在过去几十年中得到了“新的区号”,即使他们保留了同一条电话线,电话号码的最后七位数字仍然保持不变。也许你的地方从来没有出现过这种荒谬的事情,但有些人在保持同一条电话线(和相同的最后七位数字)的同时,已经更改了四次区号。 - supercat
@EricLippert:我今天早上刚在思考这个问题;存储电话号码可能比看起来更棘手。目前,如果要将数字存储在不可变类型中,我能想到的最好方法是将电话号码视为一个数字加上该数字有效的日期,然后使用一些映射表,以便414-563-XXXX Mar1990被识别为等同于920-563-XXXX Feb2012,而414-473-XXXX则被识别为等同于262-473-XXXX Feb2012。否则,如果想要在重新映射时更新数据,则可变类型似乎会有所帮助。 - supercat

8

我认为保持不可变的结构体是可以的,但个人建议除非你要在内存中同时处理大量数据,否则最好使用单独的变量来表示每个逻辑字段。如果你选择最合适的类型(例如,对于3-4位数使用ushort),那么代价不会太高,而且代码将更加清晰易懂。


我基于 DateTime 进行了设计,无论这是好事还是坏事。 - ChaosPandion
3
将5个逻辑值合并成一个从来不是一个“简单”的方法。 - Jon Skeet
2
这就是为什么它是一个 struct。如何存储结构体的数据是一个实现细节。通常情况下,如果将单独的值分开存储,你将会遇到更少的问题。尝试将它们合并在一起几乎总是会导致问题。 - kyoryu
@ChaosPandion: DateTime 将其值存储为自纪元以来的滴答数;这使得比较值变得容易,因为 DateTime 在所有 DateTime 值共享的单个时间线上逻辑上是一个瞬间。电话号码彼此之间没有关系,也不容易相互比较。您不应该基于根本不相关的概念设计实现。 - James Dunne
@JamesDunne:DateTime 实际上是一堆不同的东西,取决于它的“种类” - 但这是另一个故事 :) - Jon Skeet
显示剩余5条评论

2
也许你的同事可以通过一组方法来满足,允许单个字段轻松地“更改”(导致一个新实例与第一个实例具有相同的值,除了新字段)。
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); } }

"Empty"属性比"default(PhoneNumber)"或"new PhoneNumber()"更具自然语言风格,可使用"foo==PhoneNumber.Empty"或"foo.IsEmpty"进行等效于空检查。

此外,在您的TryParse中,您是不是想要

return result.value != 0;

我喜欢“Empty”属性的想法。但是,对于辅助方法,我不太确定。 - ChaosPandion

2

我同意这应该是一个不可变类型。但是为什么这个结构体需要实现ICloneable和IEquatable接口呢?它是一个值类型。


嗯,你说的 ICloneable 是对的,但是 DateTime 甚至实现了 IEquatable - ChaosPandion
4
实现IEquatable<PhoneNumber>是完全正确的。为什么不这样做呢?你为什么不想要能够检查一个电话号码是否等于另一个电话号码,并在集合中对它们进行哈希等操作呢? - Jon Skeet

2
个人而言,我认为将其保留为不可变结构体是非常好的。我不建议将其更改为可变类。
在我的经验中,大多数情况下,想要避免不可变结构体的人都是因为懒惰。不可变结构体强制您重新创建具有完整参数的结构体,但良好的构造函数重载可以在这方面提供极大的帮助。(例如,请查看此 Font constructor - 即使它是一个类,它也实现了“克隆除此变量之外的所有内容”的模式,您可以将其复制到需要更改的常见字段上。)
创建可变类会引入其他问题和开销,除非必要,否则我会避免使用它们。

0

通过 PhoneNumber,可以轻松处理 Nullability 问题吗?


这应该是一条注释,还是我漏掉了什么? - ChaosPandion
他们觉得没有空引用和根据需要更改对象的能力是不方便的。 - micahtan

0

将要分段可更改的数据持有者应该是结构体,而不是类。虽然人们可以辩论结构体是否应该被分段更改,但可变类不适合作为数据持有者

问题在于每个类对象实际上包含两种信息:

  1. 所有字段的内容
  2. 存在于其中的所有引用的位置

如果一个类对象是不可变的,那么存在对它的引用通常并不重要。然而,当一个数据持有的类对象是可变的时,所有对它的引用实际上都“附着”在一起;对其中任何一个进行的任何改变都将实际上对所有引用进行。

如果PhoneNumber是一个可变的结构体,那么可以更改类型为PhoneNumber的一个存储位置的字段,而不会影响该类型的任何其他存储位置中的字段。如果说var temp = Customers("Fred").MainPhoneNumber; temp.Extension = "x431"; Customers("Fred").MainPhoneNumber = temp;,那么这将更改Fred的分机号码,而不会影响其他人的。相比之下,如果PhoneNumber是一个可变的类,上述代码将设置每个MainPhoneNumber持有对同一对象的引用的人的分机号码,但不会影响持有相同数据但不是同一对象的MainPhoneNumber的任何人的分机号码。很糟糕。


网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接