2009-04-08 20 views
9

Ho esaminato i modelli di First First Design (sono appena entrato di recente) e stavo leggendo il modello di strategia, e mi è venuto in mente che potrebbe essere un ottimo modo per implementare un modo comune di calcolo delle tasse ecc. su tutti gli oggetti particolari che uso al lavoro, ma ho avuto una domanda al riguardo.Aiuto con il modello di strategia

Ecco quello che stavo pensando:

public interface ITax 
{ 
    decimal ProvincialTaxRate { get; set; } // Yes, I'm Canadian :) 
    decimal CalculateTax(decimal subtotal); 
} 

public SaskatchewanTax 
{ 
    public decimal ProvincialTaxRate { get; set; } 

    public SaskatchewanTax() 
    { 
     ProvincialTaxRate = new decimal(0.05f); 
    } 

    public decimal CalculateTax(subtotal) 
    { 
     return ProvincialTaxRate * subtotal + FederalTaxRate * subtotal; 
    } 
} 

public OntarioTax 
{ 
    public decimal ProvincialTaxRate { get; set; } 

    public OntarioTax() 
    { 
     ProvincialTaxRate = new decimal(0.08f); 
    } 

    public decimal CalculateTax(decimal subtotal) 
    { 
     return ProvincialTaxRate * subtotal + FederalTaxRate * subtotal; 
    } 
} 

Avrete notato che non v'è alcuna dichiarazione di FederalTaxRate ed è quello che volevo chiedere. Dove dovrebbe andare?

  • Passare al costruttore per ciascun calcestruzzo ITax sembra ridondante e consentirebbe un comportamento scorretto (tutti i calcolatori di imposte devono condividere la stessa identica aliquota fiscale federale).
  • Allo stesso modo, la creazione di un membro di ITax consentirebbe loro di essere incoerenti.

È necessario che tutti i calcolatori fiscali ereditino da qualche altra classe in cui è definito sia staticamente che ITax?

public class TaxCalculator 
{ 
    public static decimal FederalTaxRate = new decimal(0.05f); 
} 

risposta

21

Penso che questo sia un caso comune di abuso di pattern.

Se si controllano le due "strategie", fanno ESATTAMENTE la stessa cosa. L'unica cosa che cambia è ProvincialTaxRate.

Vorrei mantenere le cose ASCIUTTE e non utilizzare eccessivamente questo schema (o qualsiasi altro), qui guadagni un po 'di flessibilità, ma poi hai anche 2 classi che non tirano i loro pesi, e probabilmente Non hai bisogno della flessibilità.

Questo è comune quando si impara una nuova tecnologia o intuizione, si desidera applicarla ovunque (succede a ognuno di noi), anche se danneggiarne la leggibilità e la manutenibilità del codice.

La mia opinione: Keep It Simple

saluti

EDIT (In risposta al commento autore sulla mia risposta)

Non ho provato a fare gioco di te, o chiunque. Questo è un errore comune, l'ho fatto MOLTE volte, e l'ho imparato nel modo più duro, non solo con i modelli ma anche con quadri fantasiosi, server, nuove tecnologie di buzzword, come lo chiami.

Gli autori stessi del libro avvertono i lettori di non abusare di schemi, e gli upvotes in questa risposta indicano chiaramente anche qualcosa.

Ma se per qualche motivo si desidera continuare a implementare il modello, ecco il mio modesto parere:

  • Fai un superclasse per entrambe le strategie, questa superclasse sarebbe astratto e dovrebbe contenere il valore della frequenza condivisa di loro strategie bambino (FederalTaxRate)

  • Eredita e implementare il metodo astratto "Calcola" in ogni sottoclasse (qui vedrete che entrambi i metodi sono gli stessi, ma continuiamo)

  • Cerca di rendere immutabile ogni strategia concreta, favorisci sempre l'immutabilità come dice Joshua Bloch. Per questo, rimuovere il setter di ProvincialTaxRate e specificare il valore sul costruttore o direttamente nella sua dichiarazione.

  • Infine, mi piacerebbe creare alcuni metodi factory statici nella StrategySuperclass in modo da disaccoppiare le nostre clienti dalle implementazioni o strategie concrete (che può benissimo essere classi protette ora)

Modifica II : Ecco un pastie con un codice (pseudo) per rendere la soluzione un po 'più chiaro

http://pastie.org/441068

Speranza che aiuta

saluti

+0

Questo può essere vero, ma non risponde alla domanda o non mi dà alcun tipo di utilizzo se non "apparentemente" prendendomi gioco di me. –

+0

Modificato per rispondere a questo commento^ –

+0

Poiché sembra che tu stia semplicemente imparando questi schemi, non dovresti prendere le critiche di qualcuno come un attacco personale. Cercherete in ogni occasione di utilizzare questi nuovi modelli in un primo momento e talvolta li abuserete come menzionato in questa risposta. Tutto parte del processo di apprendimento. –

2

A mio parere, si ha il diritto soluzione - creare una classe di base che contiene il tasso di fax federale canadese da cui tutte le classi derivate possono ereditare. Definirlo staticamente è un'idea perfetta. Si potrebbe anche fare in modo che FederalTaxRate definisca solo una funzione di accesso per l'aliquota d'imposta, in modo da poter essere presumibilmente definita in fase di runtime da un file o altro.

Non penso che questa sia la soluzione migliore, ma funzionerà perfettamente. I modelli di progettazione non dovrebbero intralciare il buon senso e penso che il buon senso risolverà questo problema.

1

alcuni punti:

  1. ProvincialTaxRate dovrebbe quasi certamente essere immutabile a livello di interfaccia (senza set proprietà). Cambiare le aliquote fiscali non sembra una buona idea, anche se questo significa che non è possibile utilizzare le proprietà automatiche nella propria implementazione.

  2. Se c'è solo uno FederalTaxRate ed è solo un semplice valore numerico, penso che la classe base astratta sia un approccio sicuro.

  3. Meno consigliabile: A seconda di come le tasse di lavoro, si potrebbe sostenere che CalculateTax dipende FederalTaxRate e quindi questa deve essere fornito come parametro (forse ci sono diversi FederalTaxRates e non si vuole CalculateTax avere di conoscere loro).

Non lasciare che la definizione di un motivo di progettazione si intrometta in una buona idea. Sono schemi, non formule. ;)


P.S. Sono un americano, quindi se le tasse canadesi sono in realtà così semplici, spero che l'IRS tenga una pagina dal tuo libro l'anno prossimo!

2

Si potrebbe desiderare di iniziare con questo codice, e passare da lì:

public interface ITax 
{ 
    decimal CalculateTax(decimal subtotal); 
} 

public class SaskatchewanTax : ITax 
{ 
    private readonly decimal provincialTaxRate; 
    private readonly decimal federalTaxRate; 

    public SaskatchewanTax(decimal federalTaxRate) 
    { 
     provincialTaxRate = 0.05m; 
     this.federalTaxRate = federalTaxRate; 
    } 

    public decimal CalculateTax(decimal subtotal) 
    { 
     return provincialTaxRate * subtotal + federalTaxRate * subtotal; 
    } 
} 

public class OntarioTax : ITax 
{ 
    private readonly decimal provincialTaxRate; 
    private readonly decimal federalTaxRate; 

    public OntarioTax(decimal federalTaxRate) 
    { 
     provincialTaxRate = 0.08m; 
     this.federalTaxRate = federalTaxRate; 
    } 

    public decimal CalculateTax(decimal subtotal) 
    { 
     return provincialTaxRate * subtotal + federalTaxRate * subtotal; 
    } 
} 

A questo punto non ci può essere molto senso avere due oggetti strategia diversa che rappresentano il calcolo delle imposte, ma con una maggiore implementazione realistica (presumo che il calcolo delle imposte sia più complicato e varia in base alla provincia), potrebbe avere senso.

Tuttavia, si dovrebbe prendere in considerazione l'applicazione del principio "la cosa più semplice che potrebbe funzionare", e utilizzare solo il modello di strategia quando si ritiene che sia necessario.

+0

Ho avuto la "cosa più semplice che potrebbe funzionare" implementata da qualche tempo, ma mentre aggiungo sempre più il codebase, trovo che devo ripetere gran parte di questo codice in più oggetti e stava esaminando diversi modi per ridurre il codice ripetuto. –

+0

Si potrebbe considerare di inserire il codice ripetuto nella classe base (astratta) e le specifiche implementazioni (strategie) per il calcolo delle imposte nelle sottoclassi. –

2

Perché non dimenticare interfacce, e basta usare l'ereditarietà per quello che si può:

public abstract class Tax 
{ 
    protected decimal ProvincialTaxRate; // Yes, you are Canadian ;) 
    public decimal CalculateTax(decimal subtotal) 
    { 
     return ProvincialTaxRate * subtotal + FederalTaxRate * subtotal; 
    } 
    decimal FederalTaxRate = new decimal(0.20f); 
} 

public class SaskatchewanTax : Tax 
{ 
    public SaskatchewanTax() 
    { 
     base.ProvincialTaxRate = new decimal(0.05f); 
    } 

} 

public class OntarioTax : Tax 
{ 
    public OntarioTax() 
    { 
     base.ProvincialTaxRate = new decimal(0.08f); 
    } 

} 

Se è necessario l'interfaccia, basta attuarla nella base classe, e basta usare le classi derivate per comportamento/comportamento personalizzato.

0

Solo spunti di riflessione - cosa c'è di sbagliato nel mettere questo metodo in una classe appropriata, e solo chiamandolo?

public decimal CalculateTax(decimal subtotal, decimal provincialTaxRate, decimal federalTaxRate) { 
    return provincialTaxRate * subtotal + federalTaxRate * subtotal; 
    } 

Capisco che si desidera utilizzare un tasso di provincia diversa per ogni provincia, ma sicuramente che non dovrebbe essere hard-coded in un'implementazione di interfaccia?

+0

Questo potrebbe non rispondere alla domanda, ma potrebbe essere una soluzione migliore per il problema specifico. A meno che alcuni calcoli di tasse davvero funky entrino nel mix. – ftvs

0

Sono state fornite molte buone risposte. Solo per aggiungere i miei due centesimi. Quando si utilizza un modello di progettazione in questo modo:

  • creare le strategie come classi separate derivate da una classe astratta
  • Per pulire le cose: mettere tutto il codice duplicato tra strategie nella classe base astratta derivata

Se ti imbatti in una situazione in cui hai 10 schemi di strategia per le tasse che richiedono una tassa statale, e 5 utilizzando una tassa governativa, puoi fare due classi astratte derivate (es. StateTax e GovernmentTax), derivanti dalla classe astratta principale (Tasse?) E da StateTax e GovernmentTax è possibile derivare classi concrete (come OntarioTax, Te xasTax ecc.). Se successivamente è necessario un cambio di tipo di imposta, è possibile lasciarlo derivare da un'altra classe di tasse, mantenendo intatto tutto il codice generico.

Problemi correlati