Question

Après quelques semaines de lecture sur ce forum je pensais que c'était temps pour moi de faire mon premier poste.

Je suis actuellement code complet relisant. Je pense qu'il est de 15 ans depuis la dernière fois, et je trouve que je ne peux toujours pas écrire du code ;-)

Quoi qu'il en soit à la page 138 en code complet vous trouvez cet exemple d'horreur de codage. (Je l'ai retiré une partie du code)

class Emplyee {
public: 
 FullName GetName() const;
 Address GetAddress() const;
 PhoneNumber GetWorkPhone() const;
 ...

 bool IsZipCodeValid( Address address);
 ...

private: 
   ...
}

Qu'est-ce que Steve pense est mauvais est que les fonctions sont vaguement liées. Ou est-il écrit: « Il n'y a pas de lien logique entre les employés et les routines qui vérifient les codes postaux, les numéros de téléphone ou des classifications d'emploi »

Ok je suis totalement d'accord avec lui. Peut-être quelque chose comme l'exemple ci-dessous est mieux.

class ZipCode
{
public:
 bool IsValid() const;
    ...
}

class Address {
public:
   ZipCode GetZipCode() const;
   ...
}

class Employee {
public: 
 Address GetAddress() const;
    ...
}

Lors de la vérification si le zip est valide, vous devez faire quelque chose comme ça.

employee.GetAddress().GetZipCode().IsValid();

Et ce n'est pas bon en ce qui concerne la loi de Déméter .

Donc, si vous voulez supprimer deux des trois points, vous devez utiliser la délégation et quelques fonctions wrapper comme celui-ci.

class ZipCode
{
public:
 bool IsValid();
}

class Address {
public:
   ZipCode GetZipCode() const;
   bool IsZipCodeValid() {return GetZipCode()->IsValid());
}

class Employee {
public: 
 FullName GetName() const;
 Address GetAddress() const;
 bool IsZipCodeValid() {return GetAddress()->IsZipCodeValid());
 PhoneNumber GetWorkPhone() const;
}

employee.IsZipCodeValid();

Mais là encore vous avez des routines qui n'a pas de lien logique.

Je pense personnellement que les trois exemples dans ce poste sont mauvais. Est-ce une autre façon que je ne l'ai pas pensé?

Était-ce utile?

La solution

Il est maintenant payer par rapport à payer plus tard.

Vous pouvez écrire les fonctions de délégation et wrapper à l'avant (payer maintenant) et ont moins de travail changeant les entrailles de employee.IsZipCodeValid () plus tard. Ou, vous pouvez tunnel vers IsZipCodeValid en écrivant

employee.GetAddress().GetZipCode().IsValid();
partout où vous avez besoin dans le code, mais payez plus tard si vous décidez de changer votre conception de classe d'une manière qui brise ce code.

Vous pouvez choisir votre poison. ;)

Autres conseils

Il vous manque la connexion logique:

class ZipCode
{
public:
 bool IsValid();
}

class Address {
public:
   ZipCode GetZipCode() const;
   bool IsAddressValid();
   bool IsValid() {return GetZipCode()->IsValid() && IsAddressValid());
}

class Employee {
public: 
 FullName GetName() const;
 Address GetAddress() const;
 bool IsEmployeeValid();
 bool IsValid() {return GetAddress()->IseValid() && IsEmployeeValid());
 PhoneNumber GetWorkPhone() const;
}

employee.IsValid();

Comme il n'y a pas de lien logique entre la classe des employés et validation zip code, vous pouvez mettre la validation de code postal dans la classe d'adresse où il appartient plus logiquement. Ensuite, vous pouvez demander à la classe d'adresses pour valider le code postal pour vous.

class Address
{
    public:
        static IsZipValid(ZipCode zip) { return zip.isValid(); }
};

Ensuite, vous faire

Address::IsZipValid(employee.GetAddress().GetZipCode());

Je pense que cela est satisfaisant sous vos contraintes d'association logique et le droit de Déméter.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top