Which class does a better 'Separation of Concerns'

Question

I have this class, which creates a document and saves it:

public class DocCreator
{
  private IDocumentStore _documentStore;

  public DocCreator(IDocumentStore documentStore)
  {
    _documentStore = documentStore;
  }

  public void CreateAndSave()
  {
    var doc = new Document();
    doc.Title = "this is a title";
    doc.Content = whateverStream;
    doc.Hash = CalculateHash(doc.Content);
    //[do more things to create a doc]

    _documentStore.PersistToDisk(doc);
  }
}

I think it's decent, as the code to save things is hidden in DocumentStore. But we can take it one step further, and remove the call _documentStore.PersistToDisk(doc); to another class, like this:

public class DocCreatorWorkflow
{
  private IDocumentStore _documentStore;

  public DocCreatorWorkflow(IDocumentStore documentStore)
  {
    _documentStore = documentStore;
  }

  public void CreateAndSave()
  {
    var docCreator = new DocCreator();
    var doc = docCreator.Create();

    _documentStore.PersistToDisk(doc);
  }
}

In the example above I've created another class, which calls two lower classes, and so becomes responsible for the 'workflow'. It might be cleaner, but it also complicates things more. Doesn't it?

Or should I always go for the second option?

You may want to post this question on http://codereview.stackexchange.com/ it is there for reviewing code and recommending improvements and reviews — Simon Martin, Dec 01 '12 at 13:03
I think the question you should ask yourself to answer this question is: Is it a realistic scenario for your application that you will ever want to create a Document without saving it to the storage? If so then your second approach is the better idea because you can just reuse your code as it is. If not then your first approach is a valid alternative. — Dan, Dec 01 '12 at 13:20

Mickael Caruso · Accepted Answer · 2012-12-01T22:54:35.503

I would go with Option 2. You would need to modify the DocCreatorClass, though, since it is no longer responsible for saving it to disk:

public static class DocCreatorClass
{
    public static Document Create()
    {
        Document doc = new Document();
        // Property assignment code here.

        return doc;
    }
}

It would be static so that you would not need to instantiate a DocCreatorClass. I would also create separate functions for Create and Save in the DocCreatorWorkflow class:

public class DocCreatorWorkflow
{
    public IDocumentStore _documentStore;

    public DocCreateWorkflow(IDocumentStore documentStore)
    {
    }

    public void Document Create()
    {
        return DocCreatorClass.Create();
    }

    public void Save(Document doc)
    {
        _documentStore.PersistToDisk(doc);  
    }

    public void CreateAndSave()
    {
        Save(Create());
    }
}

This way, you don't always have to immediately save to disk the newly-created document. CreateAndSave() would be a convenience function that calls both Save() and Create() inside it, in case your program wants to immediately save a new document often enough.

This type of design is definitely more coding which may come across as more complicated. In the long run, it's easier to look at and maintain because each function does only one thing.

I personally stick with (most of the time, since there may be exceptions) the one class, one responsibility rule. This makes it easier to locate a part of your project when you notice that a functionality doesn't work. When you work on fixing it, you can be rest assured that the rest of your application (the other tasks, thus classes) is untouched. For functions, I like to create them in such a way that within a class, no code blocks will be repeated in two or more different places. This way, you won't have to hunt down all of those identical code blocks to update.

score 0 · Answer 2 · answered Dec 01 '12 at 17:29

Option two looks better, based on the information available (although there might be other info that may change this judgement).

But, in general, how do you determine which one is better? I think, it is better to start with conceptualizing the concerns, at first, without involving the code. For example, in this case, in my opinion, there are three concerns. 1) creating a document 2) persisting a document 3) performing the logic (some unit of work) that involves creating and saving a document. The key point is, that this third concern is separate from the first two. Neither DocCreator, nor DocumentStore, know that they are being called in this way, or some other way for that matter. Hence, it is not their concern.

Which class does a better 'Separation of Concerns'

2 Answers2