9
votes

Si cette structure immuable devrait-elle être une classe mutable?

J'ai montré cette structure à un autre programmeur et ils ont senti que cela devrait être une classe mutable. Ils ont estimé qu'il est gênant de ne pas avoir de références nulles et la possibilité de modifier l'objet selon les besoins. J'aimerais vraiment savoir s'il y a une autre raison de faire une classe mutable.

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


4 commentaires

Sauf si vous essayez d'utiliser le commutateur de la société de téléphonie locale, cela ressemble à une chaîne fortement excentrée. Je ne sais pas, je ne peux pas dire pourquoi il doit être compliqué. Qui est un problème en soi.


@nobugz - Est-ce le fait que je presse tout dans un ulong qui le rend compliqué?


Où est AreaCode.Tostring () [1] - 48> 8 vient? AreaCode.Tostring () [1] == '9' est beaucoup plus évident. Et n'oubliez pas AreaCode% 100 == 11 . Pendant que vous y êtes, vous devez savoir 37x et 96x sont également des codes de zone réservés.


@gabe - areaCode.tostring () [1] == '9' rend certainement le code plus clair.


7 Réponses :


8
votes

Je pense que c'est bien de le garder comme une structure immuable - mais j'utiliserais personnellement des variables distinctes pour chacun des champs logiques à moins que vous n'ayez énormes numéros d'entre eux en mémoire un temps. Si vous vous en tenez au type le plus approprié (par exemple, Ushort pour 3-4 chiffres), il ne doit pas être que cher - et le code sera un diable de beaucoup plus clair.


10 commentaires

Je basé sur la conception de DateTime si c'est une bonne chose ou une mauvaise chose.


Vous pensez vraiment que le code est si mauvais? J'ai senti le struct dans son ensemble était assez simple.


La combinaison de 5 valeurs logiques en une n'est jamais exactement une approche "simple".


Techniquement, il s'agit d'une valeur logique avec des sous-valeurs mais votre point est toujours debout.


C'est pourquoi c'est une struct . Comment la structure stocke ses données est un détail de mise en œuvre. Dans Général , vous trouverez que vous avez moins de problèmes si vous stockez des valeurs individuelles séparément. Essayer de les combiner semble presque toujours conduire à des problèmes.


@kyoryu - donc le consensus est que la conservation des composants séparément est la meilleure façon d'aller?


@Chaospandion: Si j'ai reçu un avis de code avec ce code, je dirais de stocker les données dans des champs distincts. Il n'y a vraiment aucun avantage à ne pas le faire. J'imaginerais que DateTime le fait à cause des détails de la mise en œuvre sous-jacents.


@Chaospandion: DateTime stocke sa valeur sous forme de chiffres depuis une époque; Cela facilite la comparaison des valeurs car un DateTime est logiquement un instant sur une seule timeline partagée par toutes les valeurs DateTime . Les numéros de téléphone ne se rapportent pas les uns aux autres et ne sont pas facilement comparables les uns aux autres. Vous ne devez pas baser votre implémentation sur la conception d'un concept fondamentalement non liée.


@Jamesdunne: une date d'heure est en fait un tas de choses différentes en fonction de son "type" - mais c'est une autre histoire :)


@Jon Skeet: En effet c'est :). J'essayais simplement de faire le point de concevoir sur la base de concepts non liés.



2
votes

Je conviens que cela devrait être un type immuable. Mais pourquoi cette structure devrait-elle mettre en œuvre une interface ICLONABLE et IEQUATIVE? C'est un type de valeur.


2 commentaires

Hmm, vous avez raison sur le iClonable mais même DateTime implémente iéquatif .


Il est absolument correct pour qu'il puisse implémenter IEQUATILABLE . Pourquoi n'est-ce pas? Pourquoi voudriez-vous pas vouloir pouvoir vérifier si un numéro de téléphone est égal à un autre et les hachage dans un ensemble, etc.?



2
votes

Personnellement, je sens que laisser cela comme une structure immuable est une très bonne chose. Je ne recommanderais pas de le changer à une classe mutable.

La plupart du temps, dans mon expérience, les personnes qui souhaitent éviter les structures immuables le font de la paresse. Les structures immuables vous oblige à recréer la structure des paramètres complets, mais de bonnes surcharges constructeurs peuvent aider énormément ici. (Pour un exemple, consultez ce constructeur de polices - même s'il est Une classe, elle implémente un "clone tout sauf ce" modèle variable "que vous pouvez dupliquer pour vos champs communs qui ont besoin de changer.)

Créer des classes mutables introduit d'autres problèmes et les frais généraux que j'éviterais sauf si nécessaire.


0 commentaires

21
votes

Un programme qui manipule un numéro de téléphone est un modèle d'un processus.

Par conséquent, faites des choses qui sont immuables dans le processus immuable dans le code. Faire des choses qui sont mutables dans le processus mutable dans le code.

Par exemple, un processus inclut probablement une personne. Une personne a un nom. Une personne peut changer son nom tout en conservant leur identité. Par conséquent, le nom d'un objet de personne devrait être mutable.

Une personne a un numéro de téléphone. Une personne peut changer son numéro de téléphone tout en conservant leur identité. Par conséquent, le numéro de téléphone d'une personne doit être mutable.

Un numéro de téléphone dispose d'un code régional. Un numéro de téléphone ne peut pas changer d'indicatif régional et conserver son identité; Vous modifiez l'indicatif régional, vous avez maintenant un numéro de téléphone différent. Par conséquent, le code de zone d'un numéro de téléphone doit être immuable.


6 commentaires

Alors, dis-tu un champ mutable sur personne avec un numéro de téléphone immuable est la mauvaise direction?


Laissez-moi réessayer. Une chaîne n'est pas mutable. Pour transformer "Anderson" en "Andreeson", vous ne modifiez pas "Anderson"; Vous le jetez entièrement et remplacez le tout. Le champ nom est mutable; La chaîne qui contient un nom donné n'est pas. Même avec un numéro de téléphone. Un numéro de téléphone n'est pas mutable; Pour transformer le 519-555-1212 en 206-555-1212, vous ne mutez pas le premier, vous le jetez et recommencez. Mais le champ de numéro de téléphone sur une personne est mutable.


Je l'ai eu, nous sommes sur la même page maintenant.


Depuis, vous êtes ici peut-être que vous pourriez donner votre avis sur la conception du struct ?


Vous dites qu'un numéro de téléphone ne peut pas changer d'indicatif régional et conserver son identité. Cela dépend de la façon dont une interprète exactement l'identité d'un numéro de téléphone. Beaucoup de personnes et d'entreprises autour d'ici disent qu'ils ont eu des "nouveaux codes de zone" au cours des deux dernières décennies, même s'ils ont conservé la même ligne téléphonique, et les sept derniers chiffres de leur numéro de téléphone sont restés les mêmes. Peut-être que vous n'aviez jamais eu de bêtises dans votre cou du bois, mais certaines personnes ont eu leur code régional change quatre fois tout en gardant la même ligne téléphonique (et les mêmes derniers chiffres).


@ERICLIPPERT: Je faisais juste réfléchir que ce matin; Stocker un numéro de téléphone est peut-être plus difficile qu'il apparaît. À ce stade, la meilleure approche que je puisse penser si les chiffres devaient être stockés dans un type immuable seraient de considérer un numéro de téléphone sous forme de numéro de numéraire plus une date de validité de la validité, puis d'utiliser des tables de mappage de manière à ce que 414-563. -Xxxx Mar1990 serait reconnu comme équivalent au 920-563-XXXX Feb2012, tandis que 414-473-XXXX seraient reconnus comme équivalents à 262-473-XXXX Feb2012. Sinon, si on voulait mettre à jour les données lorsque les choses ont été réglées, des types mutables sembleraient utiles.



2
votes

Peut-être que votre collègue pourrait être satisfait d'un ensemble de méthodes permettant aux champs individuels d'être facilement "modifiés" (résultant d'une nouvelle instance avec les mêmes valeurs que la première instance, à l'exception du nouveau champ).

return result.value != 0;


1 commentaires

J'aime l'idée de la propriété vide . Les méthodes d'assistant, cependant, je ne suis pas si sûr.



0
votes

La nullabilité peut être facilement manipulée via PhonenNumber?


2 commentaires

Cela ne devrait-il pas être un commentaire ou je manque quelque chose?


"Ils ont estimé qu'il est gênant de ne pas avoir de références nulles et la possibilité de modifier l'objet au besoin."



0
votes

Les détenteurs de données qui vont être modifiables par morceaux doivent être des structures plutôt que des classes. Bien que l'on puisse débattre de savoir si des structures devraient être modifiables par morceaux, Les classes mutables font des détenteurs de données mobles .

Le problème est que chaque objet de classe contient efficacement deux types d'informations:

  1. le contenu de tous ses champs
  2. l'endroit où toutes les références qui y existent

    Si un objet de classe est immuable, cela ne comportera généralement pas quelles références y existent. Lorsqu'un objet de classe de saisie de données est mutable, toutefois, toutes les références sont efficacement "attachées" les unes aux autres; Toute mutation effectuée sur l'un d'entre elles sera effectivement effectuée sur tous.

    Si phonenumber était une structure mutable, on pourrait modifier les champs d'un emplacement de stockage de type phonenumber sans affecter les champs dans un autre emplacement de stockage de ce type. Si on devait dire var temp = clients ("Fred"). MainPharmonenumber; temp.extension = "x431"; Les clients ("Fred"). MainPharmonNumber = Temp; qui changerait l'extension de Fred sans affecter les autres. En revanche, si phonenumber était une classe mutable, le code ci-dessus définirait l'extension pour tous ceux dont MainMonNumber a eu une référence au même objet, mais n'affecte pas l'extension de quiconque dont les MainMonNeNumber ont détenu des données identiques mais n'étaient pas le même objet. Icky.


0 commentaires