I really don't understand the SRP applied to a "real" project (really not so real but can help me to know your approach)

I find very difficult to divide and assign responsibility to my classes.

So I posted an example in pseudo code, and I hope someone can enlighten me.

I have a simple Windows form, INPUT : an excel file, a start string, an end string OUTPUT : a grid of results, that match all substring words from start to end string from the excel list.

The form has a button that load the input, search for all words that match some criteria and bind the results into a gridview.

What I should know is your approach in refactoring this simple piece of code.

private void load() 
  Open a dialog file ;
  Get result of open dialog file as a filename ;
  If (filename exists and it is valid) {
      Call Excel libraries to convert the excel data into a gridview ;
      Fill the datagridview ;
      Release the inbuilt excel resources ;

private void process() 
  If (some validation on start and end string format) {
      Construct a regex expression filter that match with start and end string;
      Create a new list with the matched results;
      Bind result data into a new datagridview;

What is your refactor approach to this piece of code?


Preliminary remark

The Single Responsibility Principle applies to classes and modules. It's not about functionality, but about reason to change (as explained in the linked article by Uncle Bob). In other words, it's not so much about responsibility of the class, but responsibility for what the class does.

Designing functions well

Your question is more about functions and the do-one-thing-and-do-it-well principle:

  • in your load function you do two very different things: finding out what to load (user interaction waiting for user) and loading the data (technical operation that could be performed in the background without waiting).
  • in your process function, you also do two different things. The first is a complex computation (the core of your processing) the second is the output of the result.

The key here is the maintainability and reuse:

  • What if you would one day decide to load a predefined file at startup, silently, without bothering the user ? With your structure, you'd need to rewrite a second load function, so no DRY anymore. And you'd then have two functions to maintain, if the data structure would change.
  • what if one day you'd migrate your app to windows 25 with a completely overhauled API (e.g. voice driven dialogs replacing the screen dialogues) ? With your structure, the UI specialists would have to go through two heterogeneous function, trying to understand what is Ui and what stays unchanged.

This is why doing one thing is so important. It's one way to benefit from separation of concerns.

But how can we achieve anything complex, if functions do one thing ?

D-O-T comes along with another key principle: have only one level of abstraction in each function. Both principles have to be considered together.

This means that you can have a function that does one complex thing like for example "loading a file chosen by the user". But this one complex thing would be decomposed in two less complex things at a lower level of abstraction, asking(UI) and loading(reading file). Each would then be implemented in lower level function that do-one-thing-and-do-it-well.

  • It's very useful in theory, but in practice i tend to overcomplicate problem, and pollute the domain model with a lot of abstraction. I think it's one of my limit – Fadelast Nov 26 '18 at 14:24
  • @fadelast we all do. Few even try to follow these principles on their first attempt to get it to work. We follow them when trying to check in something that other coders can understand. That's why refactoring is awesome. – candied_orange Apr 25 at 17:28

Not sure why it is that you are having trouble seeing the split

//responsibility : contain the data from a spreadsheet
public class SpreadSheet
    public Row[] Rows {get;set;}

//responsibility : load spreadsheets from a data source
public class SpreadSheetLoader : ISpreadSheetLoader
    public SpreadSheet Load(string filename)

//responsibility : search a spreadsheet and return the results
public class SpreadSheetSearcher
    public SearchResult SearchSpreadSheet(SpreadSheet spreadsheet, string searchstring);

//responsibility :: join the view (form) to the models (business object defined above)
public class myApp
         this.spreadSheet = this.spreadSheetLoader.Load(this.txt_filename)
         this.searchResults = this.spreadSheetSearcher(this.spreadSheet, this.txt_searchstring)
  • Very clear and concise. You have implemented the loader as an interface to read data from different sources, i suppose. So if i want to read some data from another source like an api, it is correct to modify the code in this way ? `public enum Datasource { Local, ShowDialog, Api } public class SpreadSheetLoader : ISpreadSheetLoader { public SpreadSheet Load(Datasource source) } – Fadelast Nov 26 '18 at 15:57
  • no you would just add : ApiSpreadSheetLoader : ISpreadSheetLoader – Ewan Nov 26 '18 at 16:07
  • So you have to implement "public APISpreadSheet Load(string filename) { .... } " . But what if i don'want to take any parameter ? The API for example doesn't need any filename string as input parameter – Fadelast Nov 26 '18 at 16:42
  • well that's down to choosing your interface wisely Load(string id) might be better, but surely all excel spreadsheets have a filename? – Ewan Nov 26 '18 at 16:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.