1
\$\begingroup\$

I'm looking for your opinions about ways to improve the access to data in my application. I develop web applications, using Entity Framework 6 code first, and Autofac for dependency injection.

Before you dive in the code, keep in mind that all components in my application (except for the problematic RepositoriesFactory you will see below) are registered by the IoC container. My root (Startapp) registers all components, using a .dll I developed that supplies API for DI (Autofac wrapper), that's because I don't want Autofac references in my web project. Furthermore, I want this .dll I developed to be known and used only in my root because I know this is the best practice.

My data management:

public abstract class BaseEntityObject
{
    [Key]
    public int DbIndex {get; set;}
}

public class Letter : BaseEntityObject
{
    [DataMember]
    public int Id {get; set;}

    [DataMember]
    public string Title {get; set;}

    [DataMember]
    public string Content {get; set;}

    public virtual Folder Folder {get; set;}

    [DataMember]
    public int FolderId {get; set;}

    [DataMember]
    public DateTime CreationDate {get; set;}
}

public class Folder : BaseEntityObject
{
    [DataMember]
    public int Id {get; set;}

    [DataMember]
    public string Name {get; set;}

    [DataMember]
    public List<Letter> Letters {get; set;} 
}

public IRepository<T> where T: BaseEntityObject
{
    void Add(T entity);

    void Remove(T entity);

    List<T> Get();

    T FindById(int id);

    int SaveChanges();

    void Dispose();

}


public abstract class EFRepository<T> : IRepository<T>
{
    protected readonly DbContext Context;

    public EFRepository(DbContext context)
    {
        Context = context;
    }

    public abstract List<T> Get();

    public void Add(T item)
    {
        Context.Set<T>().Add(item);
    }

    public void Remove(T item)
    {
        Context.Set<T>().Remove(item);
    }

    public void Update(T item)
    {
        Context.Entry(item).State = EntityState.Modified;
    }

    public void Dispose()
    {
        Context.Dispose();
    }

    public int SaveChanges()
    {
        return Context.SaveChanges();
    }

    public T FindById(int id)
    {
        return Context.Set<T>().Find(id); 
    }
}


public LettersRepository : EFRepository<Letter>
{
    public LettersRepository(DbContext context) : base(context) {}

    // Override for case includes will be needed in future
    public override List<T> Get()
    {
        return Context.Set<T>().ToList();
    }
}

public FoldersRepository : EFRepository<Folder>
{
    public FoldersRepository(DbContext context) : base(context) {}

    public override List<T> Get()
    {
        return Context.Set<T>().Include("Letters").ToList();
    }
}


public class DataService
{
   private IRepositoriesFactory _factory

   public DataService(IRepositoriesFactory repoFactory)
   {
       _factory = repoFactory;
   }

   public void AddFolder(Folder folder)
   {
       using (var foldersRepository = _factory.Create<FoldersRepository>())
       {
           try
           {
                foldersRepository.Add(folder)
                foldersRepository.SaveChanges();
           }
           catch(Exception)
            {
                Log ("AddFolder failed");
            }
       }
   }

   public void AddLetter(Letter letter)
   {
       using (var lettersRepository = _factory.Create<LettersRepository>())
       {
           try
           {
                foldersRepository.Add(letter)
                foldersRepository.SaveChanges();
           }
           catch(Exception)
            {
                Log ("AddLetter failed");
            }
       }
   }

   public void RemoveFolder(int id)
   {
       using (var foldersRepository = _factory.Create<FoldersRepository>())
       {
           try
           {
                var folder = foldersRepository.FindById(id);
                foldersRepository.Remove(folder)
                foldersRepository.SaveChanges();
           }
           catch(Exception)
            {
                Log ("RemoveFolder failed");
            }
       }
   }

   public void RemoveLetter(int id)
   {
       using (var lettersRepository = _factory.Create<LettersRepository>())
       {
           try
           {
                var letter = lettersRepository.FindById(id);
                lettersRepository.Remove(letter);
                lettersRepository.SaveChanges();
           }
           catch(Exception)
            {
                Log ("RemoveLetter failed");
            }
       }
   }

   public bool ChangeLetterFolder(int letterId, int destinationFolder)
   {
        using (var lettersRepository = _factory.Create<LettersRepository>())
        {
            try
            {
                var letter = lettersRepository.FindById(letterId);
                letter.FolderId = destinationFolder;
                lettersRepository.Update(letter);
                lettersRepository.SaveChanges();
            }
            catch(Exception)
            {
                Log (string.Format("Changing letter {0} folder failed", letterId));
            }
        }
   }

   public List<Letter> GetLettersAfterSpecificDate(DateTime date)
   {
       using (var lettersRepository = _factory.Create<LettersRepository>())
        {
            try
            {
                return lettersRepository.Get().Where(x => x.CreationDate < date).ToList();
            }
            catch(Exception)
            {
                Log (string.Format("Getting all letters before : {0} failed", date));
            }
        }
   }   
}

public interface IApplicationDbContext
{
    DbSet<Folder> Folders {get; set;}   
    DbSet<Letter> Letters {get; set;}
}

public ApplicationDbContext : DbContext, IApplicationDbContext
{
    public DbSet<Folder> Folders {get; set;}

    public DbSet<Letter> Letters {get; set;}

    public ApplicationDbContext () : base("name=Application.DbConntection")
    {
        Database.SetInitializer(new CreateDatabaseIfNotExists<ApplicationDbContext>());
    }
}

public interface IRepositoriesFactory
{
    IRepository<BaseEntityObject> Create<T>();
}

// I truely hate this piece of code,
// but don't know how to use dependceny injection without having a mess with 
// my references in order to register this factory
public class RepositoriesFactory : IRepositoriesFactory
{
    public IRepository<BaseEntityObject> Create<T>()
    {
        if (typeof(T) == typeof(FoldersRepository))
            return new FoldersRepository(new ApplicationDbContext());
        if (typeof(T) == typeof(LettersRepository))
            return new LettersRepository(new ApplicationDbContext());
        return null;
    }
}
\$\endgroup\$
1
\$\begingroup\$

The factory object is pretty easy. You just need a little class reflection:

public class RepositoryFactory
{
    public RepositoryFactory()
    {
        context = new ApplicationDbContext();
    }

    private ApplicationDbContext context;

    public object Create<T>()
    {
        Type repositoryType = typeof(T);
        ConstructorInfo constructor = repositoryType.GetConstructor(new Type[1] { typeof(ApplicationDbContext) });

        if (constructor == null)
            throw new InvalidOperationException("No Constructor found for " + repositoryType.FullName);

        return constructor.Invoke(new object[1] { context });
    }
}

I would also recommend added a where constraint to your type argument:

public object Create<T>() where T : IRepository<BaseEntityObject>
{
    // ...
}

Also notice that the ApplicationDbContext object is now a private field, which allows your repositories to share the same data context. This also keeps you from editing this class each time you create a new repository.


Now on to more pressing issues. I see lots of try-catch-swallow code -- code that catches exceptions and swallows them.

Please, for the sake of Future S. Peter do not do this!!

If an exception gets thrown in the data layer, let it bubble up. Don't swallow it -- even if you log it! Let the application blow sky high. If you add a record or folder and that operation fails, that is a catastrophic problem the deserves the immediate cessation of program logic.

See my related answer (along with some humor and sarcasm):

Returning status codes from business layer

\$\endgroup\$
  • \$\begingroup\$ Accepted everything you said but didn't understand how the same context will be used.. Only one context ia created inside the factory ctor? When does it get disposed of? \$\endgroup\$ – user97059 Feb 10 '16 at 5:58
  • \$\begingroup\$ @S.Peter: That brings up a good question I have about your code as well. When does the DB context get disposed? Can you post some example code of how you use the repository factory and repositories? \$\endgroup\$ – Greg Burghardt Feb 10 '16 at 15:39
  • \$\begingroup\$ In a finally block after each try catch in my data service, forgot to mention it. Erlier I seperated the context management and it looked like this: stackoverflow.com/questions/35259320/… \$\endgroup\$ – user97059 Feb 10 '16 at 18:19
1
\$\begingroup\$

You should use virtual keyword if you want to lazy load data.

[DataMember]
public virtual ICollection<Letter> Letters {get; set;}

For IRepository<T>, I would suggest changing List<T> Get(); to IList<T> Get() or IEnumeratble<T> depends on your needs.

In the abstract class EFRepository, you could create a variable _dbSetso that you don't have to repeat Context.Set<T>()

public abstract class EFRepository<T> where T : class
{
    protected readonly DbContext _context;
    protected readonly DbSet<T> _dbSet;

    public EFRepository(DbContext context)
    {
        _context = context;
        _dbSet=context.Set<T>();
    }
    ........
    .........

In my opinion, your abstract class EFRepository doesn't need to implement the interface. I would recommend you create ILettersRepository and IFoldersRepository instead and having both of them to inherit IRepository. This would give you the option to create methods that are only needed for a particular repository. For example:

public interface IFolderRepository : IRepository<Folder>{
   Folder GetFolderByName(string name);
}

public FoldersRepository : EFRepository<Folder>, IFolderRepository
{
    public FoldersRepository(DbContext context) : base(context) {}

    public override List<T> Get()
    {
        return Context.Set<T>().Include("Letters").ToList();
    }

    public Folder GetFolderByName(string name){
     //TODO
    }
}

In such way you don't even need a factory, you can directly inject your ILettersRepository and IFoldersRepository into your service code:

public class DataService
{
   private readonly IFoldersRepository _foldersRepository;
   private readonly ILettersRepository _lettersRepository;

   public DataService(IFoldersRepository foldersRepository, ILettersRepository lettersRepository)
   {
       _foldersRepository = foldersRepository;
       _lettersRepository=lettersRepository;
   }
   ..........
   .........
}
\$\endgroup\$

Your Answer

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