2010-04-06 9 views
15

Voici mon code:objet suggestion de conception orientée

class Soldier { 
public: 
    Soldier(const string &name, const Gun &gun); 
    string getName(); 
private: 
    Gun gun; 
    string name; 
}; 

class Gun { 
public: 
    void fire(); 
    void load(int bullets); 
    int getBullets(); 
private: 
    int bullets; 
} 

Je dois appeler toutes les fonctions membres de Gun sur un objet de soldat. Quelque chose comme:

soldier.gun.fire(); 

ou

soldier.getGun().load(15); 

Alors, qui est une meilleure conception? Masquage de l'objet pistolet en tant que membre privé et accès avec la fonction getGun(). Ou en faire un membre public? Ou je peux encapsuler toutes ces fonctions rendrait la mise en œuvre plus difficile:

soldier.loadGun(15); // calls Gun.load() 
soldier.fire(); // calls Gun.fire() 

Alors, quel est selon vous le meilleur?

+3

Combinez les réponses de Stephen et Frustrated. Avoir un 'get_gun()' privé qui fait le travail nécessaire pour que le soldat obtienne le pistolet, comme le montre Stephen, mais dire au * soldat * ce qu'il faut faire, pas l'arme, comme le montre Frustrated. – GManNickG

+0

@GMan - Merci, je suis d'accord. Cependant, au lieu de l'approche de @ Frustrated, dire au soldat quoi faire serait l'approche d'Austin? par exemple. soldier.Attack() au lieu de "soldier.loadGun()". – Stephen

+0

@Stephen: Oui, vous pouvez rendre l'encapsulation aussi profonde que vous le souhaitez. Ma question portait uniquement sur cette partie. Bien sûr, vous pouvez dire soldier.Attack() et à l'intérieur de cette fonction, vous pouvez vérifier si (! Gun.getBullets()) retourner ou des choses comme ça. – pocoa

Répondre

21

Je dirais aller avec votre deuxième option:

soldier.loadGun(15); // calls Gun.load() 
soldier.fire(); // calls Gun.fire() 

Dans un premier temps, il est plus de travail, mais comme le système devient plus complexe, vous pouvez constater qu'un soldat voudra faire d'autres choses avant et après avoir tiré leur arme (peut-être vérifier s'ils ont assez de munitions et ensuite crier "Die drageons !!" avant de tirer, et murmure "ça fait mal" après, et vérifiez s'ils ont besoin d'un rechargement). Il cache également aux utilisateurs de la classe Soldier les détails inutiles sur la façon dont l'arme est tirée.

+3

+1 pour votre description de la classe Soldat: P –

+1

J'aime vos exemples :) – Dinah

+2

Aussi, ne dites pas loadGun, dites prepareWeapon. De cette façon, quand votre soldat est dans un tank, il ne tâtonne pas avec son pistolet alors qu'il devrait faire tourner le canon du tank. – jmucchiello

1

Aucune règle d'or ne s'applique à 100% du temps. C'est vraiment un appel de jugement en fonction de vos besoins.

Cela dépend de la fonctionnalité que vous souhaitez masquer/interdire au pistolet d'accéder au Solider.

Si vous souhaitez avoir uniquement un accès en lecture seule au pistolet, vous pouvez renvoyer une référence const à votre propre membre.

Si vous souhaitez exposer uniquement certaines fonctionnalités, vous pouvez créer des fonctions wrapper. Si vous ne voulez pas que l'utilisateur essaie de changer les paramètres de Gun à travers le Soldier, alors faites des fonctions de wrapper. Généralement cependant, je vois le pistolet comme son propre objet et si cela ne vous dérange pas d'exposer toutes les fonctionnalités de Gun, et que cela ne vous dérange pas de permettre que les choses soient changées via l'objet Soldier, rends-le public.

Vous ne voulez probablement pas copier le pistolet, donc si vous faites une méthode GetGun(), assurez-vous que vous ne retournez pas une copie du pistolet.

Si vous voulez garder votre code simple, demandez au soldat de s'occuper du pistolet. Est-ce que votre autre code doit fonctionner directement avec le pistolet? Ou un soldat peut-il toujours savoir comment travailler/recharger son propre fusil?

+0

pistolet est privé dans l'exemple ci-dessus, il devrait être fait soit publique ou une méthode accesseur écrite comme getGun() – Fermin

+0

@Brian: J'ai besoin d'accéder à tous les membres publics de l'objet Gun du soldat. – pocoa

+0

@pocoa: Renvoyez ensuite une référence via GetGun() ou rendez le pistolet public. C'est probablement le plus facile. Quand vous voudrez modifier l'interface de votre pistolet plus tard, vous n'aurez plus besoin de changer l'interface de votre classe de soldat. –

1

Fournissez un "getGun()" ou simplement "gun()".

Imaginez un jour, vous devrez peut-être rendre cette méthode plus complexe:

Gun* getGun() { 
    if (!out_of_bullets_) { 
    return &gun_; 
    } else { 
    PullPieceFromAnkle(); 
    return &secret_gun_; 
    } 
} 

En outre, vous pouvez fournir un accesseur const afin que les gens peuvent utiliser une arme à feu const sur un soldat const:

const Gun &getGun() const { return gun_; } 
7

La loi de Demeter dirait d'encapsuler les fonctions.

http://en.wikipedia.org/wiki/Law_of_Demeter

De cette façon, si vous voulez un certain type d'interaction entre le soldat et le pistolet, vous avez un espace pour insérer le code.

Edit: Nous avons trouvé l'article pertinent du lien Wikipedia: http://www.ccs.neu.edu/research/demeter/demeter-method/LawOfDemeter/paper-boy/demeter.pdf L'exemple de Paperboy est très, très similaire à l'exemple de soldat que vous postez.

+3

Et gardez à l'esprit pour éviter de tomber dans la "Loi de Low Dot Counts" (http://haacked.com/archive/2009/07/14/law-of-demeter-dot-counting.aspx) –

+0

Great link Martinho ! Parfaitement résume mes sentiments à propos de LOD. –

+0

Dans ce cas, étant donné que "Gun" est un paramètre constructeur, je ne pense pas que cela s'applique. Il ne casse pas l'encapsulation car la classe client doit d'abord instancier le pistolet. Cela ne veut pas dire qu'une méthode Soldier :: Attack (Target * t) n'est pas une meilleure approche. – Stephen

5

En effet, cela dépend beaucoup de la quantité de contrôle que vous voulez avoir.

Pour modéliser le monde réel, vous pourriez même vouloir encapsuler complètement l'objet gun, et avoir juste une méthode soldier.attack(). La méthode soldat.attaque() verrait alors si le soldat portait un fusil, et quel était l'état de l'arme, et le tirerait ou le rechargerait si nécessaire. Ou peut-être jeter l'arme à la cible et s'enfuir, si les munitions étaient insuffisantes présents, ni opération ...

11

Tout d'abord, vous seriez violez la Law of Demeter en accédant à la Gun de l'extérieur de la classe Soldier.

Je considérerais des méthodes comme celles-ci à la place:

soldier.ArmWeapon(...); 
soldier.Attack(...); 

De cette façon, vous pouvez également mettre en œuvre votre poing, couteau, grenades, batte de baseball, chat laser, etc.

+1

+1 pour le chat laser! – FrustratedWithFormsDesigner

2

Habituellement ma décision est basée sur la nature de la classe de conteneur (dans ce cas, Soldier). Soit c'est entièrement un POD ou non. Si ce n'est pas un POD, je rends tous les membres de données privés et fournissons des méthodes d'accès. La classe est un POD seulement s'il n'a pas d'invariants (c'est-à-dire qu'il est impossible qu'un acteur externe puisse rendre son état inconsistant en modifiant ses membres). Votre classe de soldat ressemble plus à un non-POD à moi, donc j'irais à l'option de méthode d'accesseur. Si elle renvoie une référence const ou une référence régulière est votre propre décision, basée sur le comportement de fire() et les autres méthodes (si elles modifient l'état du pistolet ou non).

BTW, Bjarne Stroustrup parle un peu de cette question dans son site: http://www.artima.com/intv/goldilocks3.html

sidenote: Je sais que ce n'est pas précisément ce que vous avez demandé, mais je vous conseille de considérer également les nombreux mentions en D'autres réponses à la loi de Demeter: exposer les méthodes d'action (qui agissent sur le pistolet) au lieu de l'objet pistolet entier via une méthode getter. Puisque le soldat "a" le pistolet (il est dans sa main et il appuie sur la gâchette), il me semble plus naturel que les autres acteurs "demandent" au soldat de tirer. Je sais que cela peut être fastidieux si le pistolet a de nombreuses méthodes pour agir, mais peut-être aussi que celles-ci pourraient être regroupées dans des actions de plus haut niveau que le soldat expose.

+0

Non, Soldat n'est pas seulement une classe de conteneur. Merci pour le lien. – pocoa

3

Si vous exposez pistolet, vous permettent des choses au-delà des fonctions de membres du Gun, ce qui est probablement pas une bonne idée:

soldier.gun = anotherGun; // where did you drop your old gun? 

Si vous utilisez getGun(), les appels regarder un peu laid, mais vous pouvez ajouter des fonctions à Gun sans modifier Soldier.

Si vous encapsulez les fonctions (que je recommande) vous pouvez modifier le Pistolet ou introduire d'autres classes (dérivées) de Pistolet sans changer l'interface de Soldier.

0

Encapsule les fonctions pour fournir une interface utilisateur cohérente même si vous modifiez ultérieurement la logique.Les conventions de nommage sont à vous, mais normalement je n'utilise pas "getFoo()", mais juste "foo()" comme accesseurs et "setFoo()" comme setters.

  • Renvoyez la référence à const lorsque vous le pouvez (élément C++ effectif n ° 3).
  • Préférez consts, énumérations et inline à l'aide de numéros codés en dur (article 4)
  • fournissent des conventions de nommage unique pour vos membres privés pour les distinguer des arguments
  • Utilisez des valeurs non signées où ils ont du sens pour déplacer des erreurs à temps de compilation
  • Lorsque les valeurs const, comme les valeurs maximales, s'appliquent à une classe entière. Rends-les statiques.
  • Si vous prévoyez d'hériter, assurez-vous que vos Destructeurs sont virtuelles
  • initialize tous les membres à défaut sains d'esprit

Voici comment les classes s'occuper de cela. CodePad

#include <iostream> 
#include <string> 
#include <stdint.h> 

using namespace std; 

class Gun 
{ 
public: 
    Gun() : _bullets(0) {} 
    virtual ~Gun() {} 
    void fire() {cout << "bang bang" << endl; _bullets--;} 
    void load(const uint16_t bullets) {_bullets = bullets;} 
    const int bullets() const {return _bullets;} 

    static const uint16_t MAX_BULLETS = 17; 

protected: 
    int _bullets; 
}; 

class Soldier 
{ 
public: 
    Soldier(const string &name, const Gun &gun) : _name(name), _gun(gun) {} 
    virtual ~Soldier() {} 
    const string& name() const; 
    Gun& gun() {return _gun;} 

protected: 
    string _name; 
    Gun _gun; 
}; 


int main (int argc, char const *argv[]) 
{ 
    Gun gun; // initialize 
    string name("Foo"); 
    Soldier soldier(name, gun); 

    soldier.gun().load(Gun::MAX_BULLETS); 

    for(size_t i = 0; i < Gun::MAX_BULLETS; ++i) 
    { 
    soldier.gun().fire(); 
    cout << "I have " << soldier.gun().bullets() << " left!" << endl; 
    } 
    return 0; 
} 
Questions connexes