I've designed the below code for one of the requirement. I have to write different tests for the code and I would need feedback on where I can improve the design. Requirement: Read the customer details from csv, calculate his average expenses on different items on yearly and write to csv again. Read the customer details from csv, calculate his total expenses on faimly and write to csv again.As both the ExpenseCalculator uses CsvReaderWriter function, is it better to create the abstract class which implements csvReaderWriter functionality and derive from abstract class or is it better to use 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:\);    }}
C# - design to parse and write csv and manipulating data
c#
It's not a very good design, because it doesn't adhere to Single Responsibility Principle .ICsvReaderWriter has two responsibilities, as the name itself indicates. I wouldn't lump them together into one class. Besides, the Reader / Writer distinction is sort of idiomatic in the .NET world (think StreamReader / StreamWriter - you won't find a StreamReaderWriter in the standard library).IExpenseCalculator doesn't only calculate expenses, which is quite easy to tell because its method takes a filePath parameter. The Single Responsibility Principle states that a class should have one, and only one reason to change. Your IExpenseCalculator, however, has several potential reasons to change. It's not just if expenses are to be calculated differently, but also if the output of these calculations has to be retrieved or persisted differently... or not persisted but passed further along etc. And you can't easily unit test your ItemExpenseCalculator (to ensure that it works), because it pulls its input from the file system and pushes the output there as well.Inheritance hierarchy seems to be upside down to me, too. The interface is ICsvReaderWriter, but the implementation is simply a ReaderWriter. So the base interface is more specific than the child class, because its name already defines the concrete format, whereas the derived class sports a universal name omitting technical details.Why would the interface care about the storage format? I'd expect this to be designed the other way round: a generic IWriter interface that could be implemented by a concrete CsvWriter, or an XmlWriter, or a JsonWriter  if there is a need. See the principle of abstraction for reference. Key takeaway: the higher the level, the less detail. The lower the level, the more detail.There's also a slip-up in the interface definition:  public interface ICsvReaderWriter<T> {     IEnumerable<T> ParseCsv(string filePath);     void WriteCsv(string filePath, IEnumerable<Customer> customers); }Shouldn't that rather bevoid WriteCsv(string filePath, IEnumerable<T> objects);Naming is a bit off. If the interface is a Reader, not a Parser, why is the method named Parse? It introduces semantic noise. Synonyms are good in creative writing, but in software development we should strive for unambiguity.And a minor remark... not really a design issue, just a matter of code style.private readonly ICsvReaderWriter<Customer> _csvReaderWriter;public ItemExpenseCalculator(ICsvReaderWriter<Customer> csvReaderWriter){    this._csvReaderWriter = csvReaderWriter;}Some prefer to prefix fields with an underscore (_csvReaderWriter), some don't like it and then they have to explicitly use this to differentiate between fields and parameters of the same name. To each his own, it's a matter of taste and consistency. However doing both at once makes no sense. It's like wearing belt and braces. If you're referencing the current object explicitly with this, what purpose does the underscore serve?