Sostituire il condizionale con il refactoring del polimorfismo o simile?

Question

Ho provato a chiedere una variante di questa domanda prima. Ho avuto alcune risposte utili, ma ancora niente che mi sembrasse giusto. Mi sembra che questo non dovrebbe essere così difficile da decifrare, ma non sono in grado di trovare una soluzione semplice ed elegante. (Qui è il mio post precedente, ma per favore prova a guardare il problema qui indicato come codice procedurale per non essere influenzato dalla spiegazione precedente che sembrava portare a soluzioni molto complicate: Design pattern for cost calculator app?)

Fondamentalmente, il problema è creare una calcolatrice per le ore necessarie per progetti che possono contenere un numero di servizi. In questo caso "scrittura" e "analisi". Le ore sono calcolate in modo diverso per i diversi servizi: la scrittura viene calcolata moltiplicando una tariffa oraria "per prodotto" con il numero di prodotti, e più prodotti sono inclusi nel progetto, più bassa è la tariffa oraria, ma il numero totale di le ore vengono accumulate progressivamente (ad esempio, per un progetto di medie dimensioni si prendono entrambi i prezzi della gamma piccola e quindi si aggiungono i prezzi della gamma media fino al numero di prodotti effettivi). Considerando che per l'analisi è molto più semplice, si tratta solo di una maggiorazione per ogni gamma di dimensioni.

Come si sarebbe in grado di refactoring in una versione elegante e preferibilmente semplice orientata agli oggetti (si noti che non lo scriverei mai in questo modo in modo puramente procedurale, questo è solo per mostrare il problema in un altro modo in modo succinto).

Ho pensato in termini di modelli di fabbrica, strategia e decoratore, ma non riesco a far funzionare bene nessuno. (Ho letto Head First Design Patterns qualche tempo fa, e sia i decoratori che i modelli di fabbrica descritti hanno alcune somiglianze con questo problema, ma ho difficoltà a vederli come buone soluzioni come indicato qui. L'esempio di decoratore sembrava molto complicato lì solo per aggiungere condimenti ma forse potrebbe funzionare meglio qui, non lo so, almeno il fatto che il calcolo delle ore si accumula progressivamente mi ha fatto pensare al motivo del decoratore ... E l'esempio del modello di fabbrica del libro con la fabbrica di pizze ... .bene sembra proprio creare un'esplosione ridicola di classi, almeno nel loro esempio: ho trovato un buon uso per i modelli di fabbrica, ma non riesco a vedere come potrei usarlo qui senza ottenere un insieme di classi davvero complicato)

L'obiettivo principale sarebbe quello di dover cambiare solo in un posto (accoppiamento libero ecc.) Se dovessi aggiungere un nuovo paramete r (dì un'altra dimensione, come XSMALL, e/o un altro servizio, come "Amministrazione"). Ecco l'esempio di codice procedurale:

public class Conditional 
{ 
    private int _numberOfManuals; 
    private string _serviceType; 
    private const int SMALL = 2; 
    private const int MEDIUM = 8; 

    public int GetHours() 
    { 
     if (_numberOfManuals <= SMALL) 
     { 
      if (_serviceType == "writing") 
       return 30 * _numberOfManuals; 
      if (_serviceType == "analysis") 
       return 10; 
     } 
     else if (_numberOfManuals <= MEDIUM) 
     { 
      if (_serviceType == "writing") 
       return (SMALL * 30) + (20 * _numberOfManuals - SMALL); 
      if (_serviceType == "analysis") 
       return 20; 
     } 
     else //i.e. LARGE 
     { 
      if (_serviceType == "writing") 
       return (SMALL * 30) + (20 * (MEDIUM - SMALL)) + (10 * _numberOfManuals - MEDIUM); 
      if (_serviceType == "analysis") 
       return 30; 
     } 
     return 0; //Just a default fallback for this contrived example 
    } 
} 

Tutte le risposte sono apprezzate! (Ma come ho dichiarato nei miei post precedenti apprezzerei esempi di codice reali piuttosto che "Try this pattern", perché come ho detto, è quello che sto avendo problemi con ...) Spero che qualcuno abbia una soluzione davvero elegante a questo problema che ho pensato fin dall'inizio sarebbe stato molto semplice ...

======================== ===========================

nuova aggiunta:

apprezzo tutte le risposte finora, ma sono ancora non vedendo una soluzione veramente semplice e flessibile al problema (una cosa che pensavo non sarebbe stata molto complessa all'inizio, ma apparentemente lo è). Può anche darsi che non abbia ancora capito bene ogni risposta correttamente. Ma ho pensato di pubblicare il mio attuale tentativo di elaborarlo (con l'aiuto di leggere tutti i diversi punti di vista nelle risposte qui). Per favore dimmi se sono sulla strada giusta o no. Ma almeno ora sembra che stia iniziando a diventare più flessibile ... Posso facilmente aggiungere nuovi parametri senza dover cambiare in molti punti (credo!), E la logica condizionale è tutto in un unico posto. Ne ho alcuni in xml per ottenere i dati di base, il che semplifica parte del problema, e parte di esso è un tentativo di una soluzione di tipo strategico.

Ecco il codice:

public class Service 
{ 
    protected HourCalculatingStrategy _calculatingStrategy; 
    public int NumberOfProducts { get; set; } 
    public const int SMALL = 3; 
    public const int MEDIUM = 9; 
    public const int LARGE = 20; 
    protected string _serviceType; 
    protected Dictionary<string, decimal> _reuseLevels; 

    protected Service(int numberOfProducts) 
    { 
     NumberOfProducts = numberOfProducts; 
    } 

    public virtual decimal GetHours() 
    { 
     decimal hours = _calculatingStrategy.GetHours(NumberOfProducts, _serviceType); 
     return hours; 
    } 
} 

public class WritingService : Service 
{ 
    public WritingService(int numberOfProducts) 
     : base(numberOfProducts) 
    { 
     _calculatingStrategy = new VariableCalculatingStrategy(); 
     _serviceType = "writing"; 
    } 
} 

class AnalysisService : Service 
{ 
    public AnalysisService(int numberOfProducts) 
     : base(numberOfProducts) 
    { 
     _calculatingStrategy = new FixedCalculatingStrategy(); 
     _serviceType = "analysis"; 
    } 
} 

public abstract class HourCalculatingStrategy 
{ 
    public abstract int GetHours(int numberOfProducts, string serviceType); 

    protected int GetHourRate(string serviceType, Size size) 
    { 
     XmlDocument doc = new XmlDocument(); 
     doc.Load("calculatorData.xml"); 
     string result = doc.SelectSingleNode(string.Format("//*[@type='{0}']/{1}", serviceType, size)).InnerText; 
     return int.Parse(result); 
    } 
    protected Size GetSize(int index) 
    { 
     if (index < Service.SMALL) 
      return Size.small; 
     if (index < Service.MEDIUM) 
      return Size.medium; 
     if (index < Service.LARGE) 
      return Size.large; 
     return Size.xlarge; 
    } 
} 

public class VariableCalculatingStrategy : HourCalculatingStrategy 
{ 
    public override int GetHours(int numberOfProducts, string serviceType) 
    { 
     int hours = 0; 
     for (int i = 0; i < numberOfProducts; i++) 
     { 
      hours += GetHourRate(serviceType, GetSize(i + 1)); 
     } 
     return hours; 
    } 
} 

public class FixedCalculatingStrategy : HourCalculatingStrategy 
{ 
    public override int GetHours(int numberOfProducts, string serviceType) 
    { 
     return GetHourRate(serviceType, GetSize(numberOfProducts)); 
    } 
} 

E una forma semplice esempio che chiama (Credo che avrei potuto anche avere una classe wrapper progetto con un dizionario contenente gli oggetti di servizio, ma non ho ottenuto a quella):

public partial class Form1 : Form 
{ 
    public Form1() 
    { 
     InitializeComponent(); 
     List<int> quantities = new List<int>(); 

     for (int i = 0; i < 100; i++) 
     { 
      quantities.Add(i); 
     } 
     comboBoxNumberOfProducts.DataSource = quantities; 
    } 


    private void CreateProject() 
    { 
     int numberOfProducts = (int)comboBoxNumberOfProducts.SelectedItem; 
     Service writing = new WritingService(numberOfProducts); 
     Service analysis = new AnalysisService(numberOfProducts); 

     labelWriterHours.Text = writing.GetHours().ToString(); 
     labelAnalysisHours.Text = analysis.GetHours().ToString(); 
    } 
    private void comboBoxNumberOfProducts_SelectedIndexChanged(object sender, EventArgs e) 
    { 
     CreateProject(); 
    } 

} 

(non ero in grado di includere il codice XML perché ha ottenuto automaticamente formattati in questa pagina, ma è fondamentalmente solo un mucchio di elementi con ogni tipo di servizio, e ogni tipo di servizio contenente le misure con il le ore come valori.)

io non sono sicuro se sto solo spingendo il problema verso il file xml (avrei dovuto ancora aggiungere nuovi elementi per ogni nuovo ServiceType, e aggiungere elementi per qualsiasi nuova dimensione in ogni ServiceType se questo è cambiato .) Ma forse è impossibile da raggiungere quello che sto cercando di fare e non dover fare almeno quel tipo di cambiamento. Utilizzando un database anziché XML cambiamento sarebbe sufficiente aggiungere un campo e una fila:

ServiceType Small Medium Large

scrittura 125 100 60

Analisi 56 104 200

(Semplicemente formattato come un "tavolo" qui, anche se le colonne non sono abbastanza allineati ... io non sono il migliore in progettazione di database, però, e forse dovrebbe essere fatto in modo diverso, ma si ottiene l'idea ...)

Per favore dimmi cosa ne pensi!

Answer 1

Vorrei iniziare con un'enumerazione ProjectSize {Small, Medium, Large} e una semplice funzione per restituire l'enumerazione appropriata fornita da numberOfManuals. Da lì, avrei scritto diverso ServiceHourCalculators, il WritingServiceHourCalculator e AnalysisServiceHourCalculator (perché la loro logica è sufficientemente differente). Ognuno di essi richiederebbe un numero diManuali, un ProjectSize e restituirà il numero di ore. Probabilmente sarei creare una mappa da stringa a ServiceHourCalculator, così ho potuto dire:

ProjectSize projectSize = getProjectSize(_numberOfManuals); 
int hours = serviceMap.getService(_serviceType).getHours(projectSize, _numberOfManuals); 

In questo modo, quando ho aggiunto un nuovo formato di progetto, il compilatore avrebbe esitano a alcuni casi non gestite per ogni servizio. Non è tutto gestito in un unico luogo, ma è tutto gestito prima che compilerà di nuovo, e questo è tutto quello che serve.

Aggiornamento So Java, non C# (molto bene), quindi questo non può essere al 100% a destra, ma creare la mappa sarebbe qualcosa di simile:

Map<String, ServiceHourCalculator> serviceMap = new HashMap<String, ServiceHourCalculator>(); 
serviceMap.put("writing", new WritingServiceHourCalculator()); 
serviceMap.put("analysis", new AnalysisServiceHourCalculator()); 

Answer 2

Un buon inizio sarebbe per estrarre l'istruzione condizionale in un metodo (anche se solo un piccolo metodo) e dargli un nome davvero esplicito. Quindi estrai la logica all'interno dell'istruzione if nei propri metodi, sempre con nomi veramente espliciti. (Non preoccupatevi se i nomi dei metodi sono lunghi, a patto che facciano quello che vengono chiamati)

Scriverò questo codice ma sarebbe meglio scegliere i nomi.

Vorrei quindi spostare su metodi di refactoring più complicati e modelli. È solo quando si guarda una serie di chiamate di metodo che sembrerà opportuno iniziare ad applicare i pattern ecc.

Fai il tuo primo obiettivo per scrivere codice pulito, facile da leggere e comprendere. È facile entusiasmarsi per i pattern (parlando per esperienza) ma sono molto difficili da applicare se non si riesce a descrivere il codice esistente nelle astrazioni.

EDIT: Quindi, per chiarire - si dovrebbe mirare a ottenere la vostra istruzione if cercando in questo

if(isBox()) 
{ 
    doBoxAction(); 
} 
else if(isSquirrel()) 
{ 
    doSquirrelAction(); 
} 

Una volta fatto questo, a mio parere, allora è più facile da applicare alcuni dei modelli menzionati qui . Ma una volta che hai ancora calcoli, ecc ... nella tua dichiarazione if, allora è più difficile vedere il legno dagli alberi come sei troppo basso di un'astrazione.

Answer 3

questo è un problema comune, ci sono alcune opzioni che posso pensare. Ci sono due schemi di progettazione che vengono in mente, in primo luogo lo Strategy Pattern e in secondo luogo lo Factory Pattern. Con lo schema di strategia è possibile incapsulare il calcolo in un oggetto, ad esempio è possibile incapsulare il metodo GetHours in singole classi, ognuna rappresenterebbe un calcolo basato sulla dimensione. Una volta che abbiamo definito le diverse strategie di calcolo, facciamo il wrapping in una fabbrica. La fabbrica sarebbe responsabile della selezione della strategia per eseguire il calcolo, proprio come la tua dichiarazione if nel metodo GetHours. In ogni modo dai un'occhiata al codice qui sotto e vedi cosa ne pensi

In qualsiasi momento potresti creare una nuova strategia per eseguire un calcolo diverso. La strategia può essere condivisa tra diversi oggetti consentendo di utilizzare lo stesso calcolo in più punti. Anche la factory potrebbe elaborare dinamicamente quale strategia utilizzare in base alla configurazione, ad esempio

class Program 
{ 
    static void Main(string[] args) 
    { 
     var factory = new HourCalculationStrategyFactory(); 
     var strategy = factory.CreateStrategy(1, "writing"); 

     Console.WriteLine(strategy.Calculate()); 
    } 
} 

public class HourCalculationStrategy 
{ 
    public const int Small = 2; 
    public const int Medium = 8; 

    private readonly string _serviceType; 
    private readonly int _numberOfManuals; 

    public HourCalculationStrategy(int numberOfManuals, string serviceType) 
    { 
     _serviceType = serviceType; 
     _numberOfManuals = numberOfManuals; 
    } 

    public int Calculate() 
    { 
     return this.CalculateImplementation(_numberOfManuals, _serviceType); 
    } 

    protected virtual int CalculateImplementation(int numberOfManuals, string serviceType) 
    { 
     if (serviceType == "writing") 
      return (Small * 30) + (20 * (Medium - Small)) + (10 * numberOfManuals - Medium); 
     if (serviceType == "analysis") 
      return 30; 

     return 0; 
    } 
} 

public class SmallHourCalculationStrategy : HourCalculationStrategy 
{ 
    public SmallHourCalculationStrategy(int numberOfManuals, string serviceType) : base(numberOfManuals, serviceType) 
    { 
    } 

    protected override int CalculateImplementation(int numberOfManuals, string serviceType) 
    { 
     if (serviceType == "writing") 
      return 30 * numberOfManuals; 
     if (serviceType == "analysis") 
      return 10; 

     return 0; 
    } 
} 

public class MediumHourCalculationStrategy : HourCalculationStrategy 
{ 
    public MediumHourCalculationStrategy(int numberOfManuals, string serviceType) : base(numberOfManuals, serviceType) 
    { 
    } 

    protected override int CalculateImplementation(int numberOfManuals, string serviceType) 
    { 
     if (serviceType == "writing") 
      return (Small * 30) + (20 * numberOfManuals - Small); 
     if (serviceType == "analysis") 
      return 20; 

     return 0; 
    } 
} 

public class HourCalculationStrategyFactory 
{ 
    public HourCalculationStrategy CreateStrategy(int numberOfManuals, string serviceType) 
    { 
     if (numberOfManuals <= HourCalculationStrategy.Small) 
     { 
      return new SmallHourCalculationStrategy(numberOfManuals, serviceType); 
     } 

     if (numberOfManuals <= HourCalculationStrategy.Medium) 
     { 
      return new MediumHourCalculationStrategy(numberOfManuals, serviceType); 
     } 

     return new HourCalculationStrategy(numberOfManuals, serviceType); 
    } 
} 

Answer 4

Vorrei andare con una derivata del modello di strategia. Questo aggiunge classi aggiuntive, ma è più mantenibile nel lungo periodo. Inoltre, tenete a mente che ci sono ancora opporunities per il refactoring qui:

public class Conditional 
{ 
    private int _numberOfManuals; 
    private string _serviceType; 
    public const int SMALL = 2; 
    public const int MEDIUM = 8; 
    public int NumberOfManuals { get { return _numberOfManuals; } } 
    public string ServiceType { get { return _serviceType; } } 
    private Dictionary<int, IResult> resultStrategy; 

    public Conditional(int numberOfManuals, string serviceType) 
    { 
     _numberOfManuals = numberOfManuals; 
     _serviceType = serviceType; 
     resultStrategy = new Dictionary<int, IResult> 
     { 
       { SMALL, new SmallResult() }, 
       { MEDIUM, new MediumResult() }, 
       { MEDIUM + 1, new LargeResult() } 
     }; 
    } 

    public int GetHours() 
    { 
     return resultStrategy.Where(k => _numberOfManuals <= k.Key).First().Value.GetResult(this); 
    } 
} 

public interface IResult 
{ 
    int GetResult(Conditional conditional); 
} 

public class SmallResult : IResult 
{ 
    public int GetResult(Conditional conditional) 
    { 
     return conditional.ServiceType.IsWriting() ? WritingResult(conditional) : AnalysisResult(conditional); ; 
    } 

    private int WritingResult(Conditional conditional) 
    { 
     return 30 * conditional.NumberOfManuals; 
    } 

    private int AnalysisResult(Conditional conditional) 
    { 
     return 10; 
    } 
} 

public class MediumResult : IResult 
{ 
    public int GetResult(Conditional conditional) 
    { 
     return conditional.ServiceType.IsWriting() ? WritingResult(conditional) : AnalysisResult(conditional); ; 
    } 

    private int WritingResult(Conditional conditional) 
    { 
     return (Conditional.SMALL * 30) + (20 * conditional.NumberOfManuals - Conditional.SMALL); 

    } 

    private int AnalysisResult(Conditional conditional) 
    { 
     return 20; 
    } 
} 

public class LargeResult : IResult 
{ 
    public int GetResult(Conditional conditional) 
    { 
     return conditional.ServiceType.IsWriting() ? WritingResult(conditional) : AnalysisResult(conditional); ; 
    } 

    private int WritingResult(Conditional conditional) 
    { 
     return (Conditional.SMALL * 30) + (20 * (Conditional.MEDIUM - Conditional.SMALL)) + (10 * conditional.NumberOfManuals - Conditional.MEDIUM); 

    } 

    private int AnalysisResult(Conditional conditional) 
    { 
     return 30; 
    } 
} 

public static class ExtensionMethods 
{ 
    public static bool IsWriting(this string value) 
    { 
     return value == "writing"; 
    } 
} 

Answer 5

Non è necessario la fabbrica se i vostri sottoclassi si filtrano su ciò che vogliono far pagare per. Ciò richiede una classe di progetto per contenere i dati, se non altro:

class Project { 
    TaskType Type { get; set; } 
    int? NumberOfHours { get; set; } 
} 

Dal momento che si desidera aggiungere nuovi calcoli facilmente, è necessario un'interfaccia:

IProjectHours { 
    public void SetHours(IEnumerable<Project> projects); 
} 

E, alcune classi per implementare l'interfaccia :

class AnalysisProjectHours : IProjectHours { 
    public void SetHours(IEnumerable<Project> projects) { 
     projects.Where(p => p.Type == TaskType.Analysis) 
       .Each(p => p.NumberOfHours += 30); 
    } 
} 

// Non-LINQ equivalent 
class AnalysisProjectHours : IProjectHours { 
    public void SetHours(IEnumerable<Project> projects) { 
     foreach (Project p in projects) { 
      if (p.Type == TaskType.Analysis) { 
      p.NumberOfHours += 30; 
      } 
     } 
    } 
} 

class WritingProjectHours : IProjectHours { 
    public void SetHours(IEnumerable<Project> projects) { 
     projects.Where(p => p.Type == TaskType.Writing) 
       .Skip(0).Take(2).Each(p => p.NumberOfHours += 30); 
     projects.Where(p => p.Type == TaskType.Writing) 
       .Skip(2).Take(6).Each(p => p.NumberOfHours += 20); 
     projects.Where(p => p.Type == TaskType.Writing) 
       .Skip(8).Each(p => p.NumberOfHours += 10); 
    } 
} 

// Non-LINQ equivalent 
class WritingProjectHours : IProjectHours { 
    public void SetHours(IEnumerable<Project> projects) { 
     int writingProjectsCount = 0; 
     foreach (Project p in projects) { 
      if (p.Type != TaskType.Writing) { 
      continue; 
      } 
      writingProjectsCount++; 
      switch (writingProjectsCount) { 
       case 1: case 2: 
       p.NumberOfHours += 30; 
       break; 
       case 3: case 4: case 5: case 6: case 7: case 8: 
       p.NumberOfHours += 20; 
       break; 
       default: 
       p.NumberOfHours += 10; 
       break; 
      } 
     } 
    } 
} 

class NewProjectHours : IProjectHours { 
    public void SetHours(IEnumerable<Project> projects) { 
     projects.Where(p => p.Id == null).Each(p => p.NumberOfHours += 5); 
    } 
} 

// Non-LINQ equivalent 
class NewProjectHours : IProjectHours { 
    public void SetHours(IEnumerable<Project> projects) { 
     foreach (Project p in projects) { 
      if (p.Id == null) { 
      // Add 5 additional hours to each new project 
      p.NumberOfHours += 5; 
      } 
     } 
    } 
}  

il codice chiamante può o caricare dinamicamente IProjectHours implementatori (o statica) e poi solo a piedi l'elenco dei Project s attraverso la m:

foreach (var h in AssemblyHelper.GetImplementors<IProjectHours>()) { 
    h.SetHours(projects); 
} 
Console.WriteLine(projects.Sum(p => p.NumberOfHours)); 
// Non-LINQ equivalent 
int totalNumberHours = 0; 
foreach (Project p in projects) { 
    totalNumberOfHours += p.NumberOfHours; 
} 
Console.WriteLine(totalNumberOfHours);