C # - design per analizzare e scrivere csv e manipolare i dati [chiuso]

2

Ho progettato il codice seguente per uno dei requisiti. Devo scrivere diversi test per il codice e avrei bisogno di feedback su dove posso migliorare il design.

Requisito:

  1. Leggi i dettagli del cliente da csv, calcola le sue spese medie su diversi articoli su base annua e scrivi di nuovo su csv.
  2. Leggi i dettagli del cliente da csv, calcola le sue spese totali su faimly e scrivi di nuovo a csv.

Poiché sia ExpenseCalculator utilizza la funzione CsvReaderWriter, è meglio creare la classe astratta che implementa la funzionalità csvReaderWriter e deriva dalla classe astratta o è meglio usare Interface?

    public class Customer
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public int Expenses { get; set; }
    // other details
}

public interface ICsvReaderWriter<T>
{
    IEnumerable<T> ParseCsv(string filePath);
    void WriteCsv(string filePath, IEnumerable<Customer> customers);
}

public class ReaderWriter : ICsvReaderWriter<Customer>
{
    public IEnumerable<Customer> ParseCsv(string filePath)
    {
        throw new NotImplementedException();
    }

    public void WriteCsv(string filePath, IEnumerable<Customer> customers )
    {
        throw new NotImplementedException();
    }
}

public interface IExpenseCalculator
{
    void CalculateExpenses(string filePath);
}

public class ItemExpenseCalculator : IExpenseCalculator
{
    private readonly ICsvReaderWriter<Customer> _csvReaderWriter;
    public ItemExpenseCalculator(ICsvReaderWriter<Customer> csvReaderWriter)
    {
        this._csvReaderWriter = csvReaderWriter;
    }

    public void CalculateExpenses(string filePath)
    {
        var customers = _csvReaderWriter.ParseCsv(filePath);
        // do manipulation
        _csvReaderWriter.WriteCsv(filePath, customers);
    }
}

public class FamilyExpenseCalculator: IExpenseCalculator
{
    private readonly ICsvReaderWriter<Customer> _csvReaderWriter;

    public FamilyExpenseCalculator(ICsvReaderWriter<Customer> csvReaderWriter)
    {
        this._csvReaderWriter = csvReaderWriter;
    }

    public void CalculateExpenses(string filePath)
    {
        var customers = _csvReaderWriter.ParseCsv(filePath);
        // do manipulation
        _csvReaderWriter.WriteCsv(filePath, customers);
    }
}

public class Program
{
    private void Main()
    {
        IExpenseCalculator itemExpenseCalculator = new ItemExpenseCalculator(new ReaderWriter());
        itemExpenseCalculator.CalculateExpenses(@"D:\");

        IExpenseCalculator familyExpenseCalculator = new FamilyExpenseCalculator(new ReaderWriter());
        familyExpenseCalculator.CalculateExpenses(@"D:\");
    }
}
    
posta SabVenkat 14.08.2015 - 12:37
fonte

1 risposta

6
  1. Non è un ottimo design, perché non aderisce al principio di responsabilità singola .

    • ICsvReaderWriter ha due responsabilità, come indica il nome stesso. Non li metterei insieme in una classe. Inoltre, la distinzione Lettore / Scrittore è una sorta di idiomatico nel mondo .NET (si pensi a StreamReader / StreamWriter - non si troverà un StreamReaderWriter nella libreria standard).

    • IExpenseCalculator non calcola solo le spese, il che è abbastanza facile da dire perché il suo metodo accetta un parametro filePath . Il principio di responsabilità unica afferma che una classe dovrebbe avere una, e una sola ragione per cambiare. Il tuo IExpenseCalculator , tuttavia, ha diversi potenziali motivi per cambiare. Non è solo se le spese devono essere calcolate in modo diverso, ma anche se l'output di questi calcoli deve essere recuperato o mantenuto in modo diverso ... o non persistito, ma passato oltre, ecc. E non puoi facilmente testare il tuo ItemExpenseCalculator (per assicurarsi che funzioni), perché estrae il suo input dal file system e spinge anche l'output.

  2. La gerarchia di ereditarietà sembra essere capovolta anche a me. L'interfaccia è ICsvReaderWriter , ma l'implementazione è semplicemente un ReaderWriter . Quindi l'interfaccia di base è più specifica della classe figlia, perché il suo nome definisce già il formato concreto, mentre la classe derivata ha un nome universale che omette i dettagli tecnici.

    Perché l'interfaccia dovrebbe interessarsi al formato di archiviazione? Mi aspetto che questo venga progettato al contrario: un'interfaccia generica IWriter che potrebbe essere implementata da un% di% di co_de concreto, o un% diCsvWriter o un% diXmlWriter se ce n'è bisogno.

    Per riferimento, consulta il principio di astrazione . Key takeaway: "più alto è il livello, meno dettagli. Più basso è il livello, più dettagli."

  3. C'è anche un errore nella definizione dell'interfaccia:

     public interface ICsvReaderWriter<T>
     {
         IEnumerable<T> ParseCsv(string filePath);
         void WriteCsv(string filePath, IEnumerable<Customer> customers);
     }
    

    Non dovrebbe essere piuttosto

    void WriteCsv(string filePath, IEnumerable<T> objects);
    
  4. La denominazione è un po 'spenta. Se l'interfaccia è un JsonWriter , non un Reader , perché il metodo si chiama Parser ? Introduce il rumore semantico. I sinonimi sono buoni nella scrittura creativa, ma nello sviluppo del software dovremmo sforzarci per l'ambiguità.

  5. E un'osservazione minore ... non proprio un problema di progettazione, solo una questione di stile del codice.

    private readonly ICsvReaderWriter<Customer> _csvReaderWriter;
    
    public ItemExpenseCalculator(ICsvReaderWriter<Customer> csvReaderWriter)
    {
        this._csvReaderWriter = csvReaderWriter;
    }
    

    Alcuni preferiscono prefissare i campi con un carattere di sottolineatura ( Parse ), ad alcuni non piace e poi devono usare esplicitamente _csvReaderWriter per distinguere tra campi e parametri con lo stesso nome. A ciascuno il suo, è una questione di gusto e coerenza. Tuttavia, fare entrambe le cose contemporaneamente non ha senso. È come indossare cintura e bretelle. Se stai facendo riferimento all'oggetto corrente in modo esplicito con this , quale scopo serve il carattere di sottolineatura?

risposta data 14.08.2015 - 14:19
fonte

Leggi altre domande sui tag