2010-04-06 21 views
15

Ecco il mio codice:oggetto di design orientato suggerimento

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

ho bisogno di chiamare tutte le funzioni membro di pistola nel corso di un oggetto Soldato. Qualcosa di simile:

soldier.gun.fire(); 

o

soldier.getGun().load(15); 

Quindi qual è un progetto migliore? Nascondere l'oggetto pistola come membro privato e accedervi con la funzione getGun(). O renderlo un membro pubblico? O posso incapsulare tutte queste funzioni renderebbe l'implementazione più difficile:

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

Quindi quale pensi sia il migliore?

+3

Combina le risposte di Stephen e Frustrato. Avere un privato 'get_gun()' che funziona necessario per il soldato per ottenere la pistola, come Stephen mostra, ma dire al * soldato * cosa fare, non la pistola, come mostra frustrato. – GManNickG

+0

@GMan - Grazie, sono d'accordo. Tuttavia, invece dell'approccio di @ Frustrated, dire al soldato cosa fare sarebbe l'approccio di @ Austin? per esempio. soldier.Attack() invece di "soldier.loadGun()". – Stephen

+0

@Stephen: Sì, puoi rendere l'incapsulamento più profondo che vuoi. La mia domanda riguardava solo questa parte. Naturalmente puoi dire soldato.Attacco() e all'interno di questa funzione puoi controllare se (! Gun.getBullets()) tornare o cose del genere. – pocoa

risposta

21

direi andare con il vostro seconda opzione:

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

Inizialmente si è più lavoro, ma come il il sistema diventa più complesso, potresti scoprire che un soldato vorrà fare altre cose prima e dopo aver sparato con la pistola (forse controlla se ha abbastanza munizioni e poi urla "Muori schifo !!" prima di sparare, e borbotta "questo deve far male" dopo, e controllare se hanno bisogno di una ricarica). Nasconde inoltre agli utenti della classe Soldier i dettagli superflui su come viene sparata esattamente la pistola.

+3

+1 per la descrizione della classe Soldier: P –

+1

Adoro i tuoi esempi :) – Dinah

+2

Inoltre, non dire loadGun, ad esempio prepareWeapon. In questo modo quando il tuo soldato è in una cisterna, non sta armeggiando con la sua pistola quando dovrebbe ruotare il cannone del carro armato. – jmucchiello

1

Non esiste una regola d'oro che si applica al 100% del tempo. È davvero una sentenza in base alle tue esigenze.

Dipende da quanta funzionalità si desidera nascondere/non consentire alla pistola di accedere a Solider.

Se si desidera solo avere accesso in sola lettura alla pistola, è possibile restituire un riferimento const al proprio membro.

Se si desidera esporre solo determinate funzionalità, è possibile eseguire le funzioni wrapper. Se non si desidera che l'utente tenti di cambiare le impostazioni della pistola attraverso il Soldato, quindi eseguire le funzioni wrapper.

In genere, tuttavia, vedo la pistola come se fosse un oggetto proprio e se non ti dispiace esporre tutte le funzionalità di Gun e non preoccuparti di consentire che le cose vengano cambiate tramite l'oggetto Soldato, rendilo pubblico.

Probabilmente non vuoi copiare la pistola, quindi se fai un metodo GetGun() assicurati di non restituire una copia della pistola.

Se si desidera mantenere il proprio codice semplice, fare in modo che il soldato sia responsabile della gestione della pistola. Il tuo altro codice deve lavorare direttamente con la pistola? O un soldato può sempre sapere come funziona/ricaricare la propria pistola?

+0

è privata nell'esempio precedente, dovrebbe essere resa pubblica o un metodo di accesso scritto come getGun() – Fermin

+0

@Brian: Devo accedere a tutti i membri pubblici dell'oggetto Gun dal Soldato. – pocoa

+0

@pocoa: quindi restituire un riferimento tramite GetGun() o rendere pubblica la pistola. Questo è probabilmente il più semplice. Quando si desidera apportare modifiche all'interfaccia di Gun in un secondo momento, non è necessario modificare anche l'interfaccia della classe dei soldati. –

1

Fornire un "getGun()" o semplicemente "gun()".

Immaginate un giorno potrebbe essere necessario fare quel metodo più complesso:

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

Inoltre, si consiglia di fornire una funzione di accesso const così la gente può usare una pistola const su un soldato const:

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

La legge di Demetra direbbe di incapsulare le funzioni.

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

In questo modo, se si desidera che un certo tipo di interazione tra il soldato e la pistola, avete uno spazio per inserire il codice.

Edit: Trovato l'articolo in questione dal link Wikipedia: http://www.ccs.neu.edu/research/demeter/demeter-method/LawOfDemeter/paper-boy/demeter.pdf L'esempio Paperboy è molto, molto simile all'esempio soldato che pubblichi.

+3

E ricordarsi di evitare di cadere nella "Legge dei Conti Low Dot" (http://haacked.com/archive/2009/07/14/law-of-demeter-dot-counting.aspx) –

+0

Gran link Martinho ! Riassume perfettamente i miei sentimenti riguardo a LOD. –

+0

In questo caso, dato "Gun" è un parametro costruttore, non penso che questo sia valido. Non rompe l'incapsulamento dal momento che la classe cliente ha bisogno di istanziare la pistola per cominciare. Questo non vuol dire che un metodo Soldier :: Attack (Target * t) non sia un approccio migliore. – Stephen

5

In effetti, dipende molto dalla quantità di controllo che si desidera avere.

Per modellare il mondo reale, si potrebbe persino voler incapsulare completamente l'oggetto della pistola e avere solo un metodo di soldato.attacco(). Il metodo di soldato.attacco() vedrebbe quindi se il soldato stava portando una pistola, e qual era lo stato della pistola, e sparare o ricaricare se necessario. O forse gettare la pistola contro il bersaglio e fuggire, se le munizioni sufficienti erano presenti sia per il funzionamento ...

11

Prima di tutto, devi essere violando la Law of Demeter accedendo alla Gun dall'esterno della classe Soldier.

vorrei prendere in considerazione metodi come questi, invece:

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

In questo modo si potrebbe anche implementare il pugno, coltello, granata, mazza da baseball, gatto laser, ecc

+1

+1 per il gatto laser! – FrustratedWithFormsDesigner

2

Di solito la mia decisione si basa su la natura della classe contenitore (in questo caso, Soldato). O è interamente un POD o non lo è. Se non è un POD, faccio in modo che tutti i membri dei dati siano privati ​​e forniscano metodi di accesso. La classe è un POD solo se non ha invarianti (cioè non è possibile che un attore esterno possa rendere il suo stato incoerente modificando i suoi membri). La tua classe di soldato sembra più un non POD per me, quindi andrei all'opzione del metodo accessor. Se restituisce un riferimento const o un riferimento regolare è una tua decisione, basata sul comportamento di fire() e gli altri metodi (se modificano lo stato della pistola o meno).

BTW, Bjarne Stroustrup parla un po 'su questo problema nel suo sito: http://www.artima.com/intv/goldilocks3.html

sidenote: io so che non è esattamente quello che hai chiesto, ma io vi piacerebbe consiglio di prendere in considerazione anche le tante menzioni fatto in altre risposte alla legge di Demeter: esporre i metodi di azione (che agiscono sulla pistola) anziché l'intero oggetto della pistola tramite un metodo getter. Dato che il soldato "ha" la pistola (è nella sua mano e preme il grilletto), mi sembra più naturale che gli altri attori "chiedano" al soldato di sparare. So che questo può essere noioso se la pistola ha molti metodi per agire, ma forse anche questi potrebbero essere raggruppati in più azioni di alto livello che il soldato espone.

+0

No, Soldier non è solo una classe container. Grazie per il link. – pocoa

3

Se si espone pistola, è lasciare che le cose al di là delle funzioni membro della pistola, che probabilmente non è una buona idea:

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

Se si utilizza getGun(), le chiamate sembrano un po 'brutto, ma puoi aggiungere funzioni a Gun senza modificare Soldier.

Se si incapsulano le funzioni (che consiglio), è possibile modificare la pistola o introdurre altre classi (derivate) di pistola senza modificare l'interfaccia in Soldato.

0

Incapsulare le funzioni per fornire un'interfaccia utente coerente anche se in seguito si modifica la logica.Le convenzioni di denominazione dipendono da te, ma normalmente non uso "getFoo()", ma solo "foo()" come accessor e "setFoo()" come setter.

  • return reference-to-const quando è possibile (efficace articolo C++ n. 3).
  • Preferisco const, enum, e inline per utilizzare i numeri hard coded (4 # articolo)
  • forniscono convenzioni di denominazione uniche per i soci privati ​​per distinguerle dalle argomentazioni
  • Usa i valori senza segno dove fanno senso per spostare gli errori tempo di compilazione
  • Quando i valori const, come i massimi, si applicano a un'intera classe. Rendili statici.
  • Se avete intenzione di ereditare, assicurarsi che i distruttori sono virtuali
  • inizializzare tutti i membri di default sensati

In questo modo le classi di guardare dopo. 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; 
} 
Problemi correlati