3
\$\begingroup\$

So I have a program that visualizes data from files (csv,txt,etc.). These files are saved to database as individual tables. In my application I have DocumentExplorer which is just a module with a list of available documents(files). You can open and close any document you want, and you can have multiple documents opened at the same time.

This program also uses MEF to separate modules and exposes API module for users to create plugins. What I'm working on is some kind of manager to manage all opened documents in current session. Basically I need something that will be responsible for closing, opening, switching documents. I have 2 ideas how to deal with it, but I'm not sure whether they are any good or not. This is what I came up with so far:

1. DocumentManager passed by IoC container with interface exposed inside API module.

   public class DocumentManager : IDocumentManager
    {
        private List<IDocument> ActiveDocuments { get; set; }

        public void Create(IDocument document)
        {
            if(!ActiveDocuments.Contains(document))
                ActiveDocuments.Add(document);
        }

        public void Close(IDocument document)
        {
            if (ActiveDocuments.Contains(document))
                ActiveDocuments.Remove(document);
        }

        public IDocument Get(Expression<Func<IDocument, bool>> predicate)
        {
            return ActiveDocuments.AsQueryable().FirstOrDefault(predicate);
        }
    }

2. DocumentManager as static class kept inside API module, so others can access it. (without IDocumentManager interface, obviously)

What are your thoughts? Are these good approaches? Do you know any good tricks with this kind of problem?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Manager - Repository or Factory or both?

What is a Manager? It's usually a class that does too much. In this case it tries to be a repository and a factory a the same time. This should be separated.

Why would we do this? To make the usage more obvious and to be able to add functionality to either class without affecting or breaking the others and of course to test each module separately.


DocumentRepository

When implementing the repository consider switching from the List<T> to the HashSet<T>. This would improve not only the performance (which is probably neglectable here - with a dozen of documents no one will notice the change) but it would greaty simplify your code and both methods would be single calls:

public bool Add(IDocument document)
{
    return ActiveDocuments.Add(document);
}

public bool Remove(IDocument document)
{
    remove ActiveDocuments.Remove(document);
}

What about the naming? You call your methods Create and Close but I don't see anything but adding and removing documents to/from a collection. I suggest naming them simply Add and Remove respectively.

If you store the documents in a database then you may add other methods to it. However even better would be to implement two repositories. The first one only knows how to talk to the database and if you need to cache the documents then the decorator repository can do the rest.

Example:

interface IDocumentRepository
{
    IDocument Get(int id);
    bool Add(IDocument document);    
}

class DocumentRepository : IDocumentRepository
{
    public IDocument Get(int id) 
    {
        // get from database
    }
    bool Add(IDocument document)
    {
        // add to database
    }
}

class CachedDocumentRepository : IDocumentRepository
{
    private readonly DocumentRepository _documentRepository;

    private readonly HashSet<IDocument> _cache = new HashSet<IDocument>();

    public CachedDocumentRepository(DocumentRepository documentRepository)
    {
        _documentRepository = documentRepository;
    }

    public IDocument Get(int id) 
    {
        // check the cache and get a document from there or
        // get it from _documentRepository.Get(id);
        // and add to the cache
    }

    bool Add(IDocument document)
    {
        // add to the cache and
        // add to database
    }
}

IEnumerable

With your current implemetation you could remove the Get method and further simplify the repository by implementing the IEnumerable<IDocument> interface for this class.

DocumentFactory

This class should only know how to create documents. It can have methods for creating many different types but nothing else.

\$\endgroup\$
5
  • \$\begingroup\$ So would it be a good practice to rename it to DocumentRepository and add method Save that would send ActiveDocuments to database? I thought that managers should be responsible for managing (saving, removing, creating, changing) that's why I named it DocumentManager EDIT: Close and Create is because I later want to check if document has unsaved changes and probably other things. \$\endgroup\$
    – FCin
    Commented Nov 21, 2016 at 8:40
  • \$\begingroup\$ @FCin a Manager usually means I don't know how to name it better ;-D and it's usually class that does too much (like this example). A repository stores or privdes access to a data storage - depends on the concrete requirements whereas a factory is responsible for creating things. So actually you need both a DocumentRepository for retrieving documents and for adding them from/to an internal collection or a database and a DocumentFactory for creating documents. Those two concerns need be separated. \$\endgroup\$
    – t3chb0t
    Commented Nov 21, 2016 at 8:46
  • \$\begingroup\$ Thank you for your help. If I understand correctly I should first create document with DocumentFactory and then add it to database via DocumentRepository? \$\endgroup\$
    – FCin
    Commented Nov 21, 2016 at 8:56
  • \$\begingroup\$ @FCin exactly. The repository shouldn't know anything about creating documents. All it cares about is getting and saving them. The factory knows how to create various documents and you probably have many of them because of the IDocument abstraction. This way you can test each of them separately and make changes to any of them without breaking the other one. \$\endgroup\$
    – t3chb0t
    Commented Nov 21, 2016 at 8:58
  • \$\begingroup\$ @FCin I've added one more example about how you could solve the caching by implementing a decorator for the normal repository. \$\endgroup\$
    – t3chb0t
    Commented Nov 21, 2016 at 9:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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