2

In conjunction with Unit Tests and Dependency Injection I (and my primary coworker) are exploring Repositories. However we cannot come to a solid plan of action for implementation.

In a basic scenario we have a Repository that encapsulates a single context and one or more entities. The public methods of that Repository return either List or a single entity result. IE

public class SomeEntity
{
    public int Id { get; set; }
    public string SomeValue { get; set; }
}

public class SomeContext : DbContext
{
    public virtual DbSet<SomeEntity> SomeEntities { get; set; }
}

public class SomeRepo
{
    public SomeRepo()
    {
        _context = new SomeContext();
    }

    public SomeEntity GetEntity(int id)
    {
        return _context.SomeEntities.Find(id);
    }

    public IQueryable<SomeEntity> GetAllEntities()
    {
        return _context.SomeEntities;
    }
}

This is all well and good and works fine for us 99% of the time. The quandry is when there are multiple entities in a Repo and a join is required. Currently we just do something like the below in a UOW class that uses the repository;

public SomeModel SomeMethod()
{
    var entity1 = _repo.GetEntity1();
    var entity2 = _repo.GetEntity2();
    return from a in entity1
           join b in entity2 on a.id equals b.id
           select new SomeModel
           {
               Foo = a.foo,
               Bar = b.bar
           };
}

From the many contradictory discussions/posts/blogs/etc about Repositories and our own personal feelings, this doesn't seem right. What also doesn't seem right is doing the join inside the repo and then returning something that isn't one of the entities.

Our typical design is to have a context wrapped inside an Repository which is Dependency Injected into a UOW class. This way we can have a Unit Test that mocks the Repo returning fake DB results.

Knowing that this is a loaded question, what might be a good pattern for us?


For a more real world example of a join scenario (I am not happy with this code, it was a rush job to make something happen, but it's a good example of the scenario we need to address):

public class AccountingContext : DbContext
{
    public DbSet<Vendor> Vendors { get; set; }
    public DbSet<Check> Checks { get; set; }
    public DbSet<ApCheckDetail> CheckDetails { get; set; }
    public DbSet<Transaction> Transactions { get; set; }

    public AccountingContext(string connString) : base(connString)
    {

    }
}

public class AccountingRepo : IAccountingRepo
{
    private readonly AccountingContext _accountingContext;

    public AccountingRepo(IConnectionStringMaker connectionStringMaker, ILocalConfig localConfig)
    {
        // code to generate connString

        _accountingContext = new AccountingContext(connString);
    }

    public IEnumerable<Check> GetChecksByDate(DateTime checkDate)
    {
        return _accountingContext.Checks
            .Where(c => c.CHKDATE.Value == checkDate.Date &&
                        !c.DELVOIDDATE.HasValue);
    }

    public IEnumerable<Vendor> GetVendors(IEnumerable<string> vendorId)
    {
        return _accountingContext.Vendors
            .Where(v => vendorId.Contains(v.VENDCODE))
            .Distinct();
    }

    public IEnumerable<ApCheckDetail> GetCheckDetails(IEnumerable<string> checkIds)
    {
        return _accountingContext.CheckDetails
            .Where(c => checkIds.Contains(c.CheckId));
    }

    public IEnumerable<Transaction> GetTransactions(IEnumerable<string> tranNos, DateTime checkDate)
    {
        var ids = tranNos.ToList();
        var sb = new StringBuilder();
        sb.Append($"'{ids.First()}'");
        for (int i = 1; i < ids.Count; i++)
        {
            sb.Append($", '{ids[i]}'");
        }

        var sql = $"Select TranNo = TRANNO, InvoiceNo = INVNO, InvoiceDate = INVDATE, InvoiceAmount = INVAMT, DiscountAmount = DISCEARNED, TaxWithheld = OTAXWITHAMT, PayDate = PAYDATE from APTRAN where TRANNO in ({sb})";
        return _accountingContext.Set<Transaction>().SqlQuery(sql).ToList();
    }
}

public class AccountingInteraction : IAccountingInteraction
{
    private readonly IAccountingRepo _accountingRepo;

    public AccountingInteraction(IAccountingRepo accountingRepo)
    {
        _accountingRepo = accountingRepo;
    }

    public IList<CheckDetail> GetChecksToPay(DateTime checkDate, IEnumerable<string> excludeVendCats)
    {
        var todaysChecks = _accountingRepo.GetChecksByDate(checkDate).ToList();

        var todaysVendors = todaysChecks.Select(c => c.APCODE).Distinct().ToList();
        var todaysCheckIds = todaysChecks.Select(c => c.CheckId).ToList();

        var vendors = _accountingRepo.GetVendors(todaysVendors).ToList();
        var apCheckDetails = _accountingRepo.GetCheckDetails(todaysCheckIds).ToList();
        var todaysCheckDetails = apCheckDetails.Select(a => a.InvTranNo).ToList();

        var tranDetails = _accountingRepo.GetTransactions(todaysCheckDetails, checkDate).ToList();


        return (from c in todaysChecks
                join v in vendors on c.APCODE equals v.VENDCODE
                where !c.DELVOIDDATE.HasValue &&
                      !excludeVendCats.Contains(v.VENDCAT) &&
                      c.BACSPMT != 1 &&
                      v.DEFPMTTYPE == "CHK"
                select new CheckDetail
                {
                    VendorId = v.VENDCODE,
                    VendorName = v.VENDNAME,
                    CheckDate = c.CHKDATE.Value,
                    CheckAmount = c.CHKAMT.Value,
                    CheckNumber = c.CHECKNUM.Value,
                    Address1 = v.ADDR1,
                    Address2 = v.ADDR2,
                    City = v.CITY,
                    State = v.STATE,
                    Zip = v.ZIP,
                    Company = c.COMPNUM.Value,
                    VoidDate = c.DELVOIDDATE,
                    PhoneNumber = v.OFFTELE,
                    Email = v.EMAIL,
                    Remittances = (from check in todaysChecks
                                   join d in apCheckDetails on check.CheckId equals d.CheckId
                                   join t in tranDetails on d.InvTranNo equals t.TranNo
                                   where check.CheckId == c.CheckId
                                   select new RemittanceModel
                                   {
                                       InvoiceAmount = t.InvoiceAmount,
                                       CheckAmount = d.PaidAmount,
                                       InvoiceDate = t.InvoiceDate,
                                       DiscountAmount = t.DiscountAmount,
                                       TaxWithheldAmount = t.TaxWithheld,
                                       InvoiceNumber = t.InvoiceNo
                                   }).ToList()
                }).ToList();
    }
}
  • Joins are for correlating data sets with multiple records. That's not what you have here. Do you have a more concrete, real-world example you can provide? – Robert Harvey Jul 18 '18 at 21:10
  • Side note, I would inject the context to the repository using DI, instead of instantiating a concrete class in the constructor. – bobek Jul 18 '18 at 21:11
  • OK, so you've misnamed your methods then? Assuming they should really say GetEntities(), is your problem performance? Or is it something else? – Robert Harvey Jul 18 '18 at 21:18
  • @RobertHarvey I've added a more real world example instead of a demo. – gilliduck Jul 18 '18 at 21:23
  • @RobertHarvey, no misnaming. The example with the method showing the join is just using names not already laid out in the earlier example. Performance isn't really the main issue, but we certainly don't want to return a couple of lists with million items in each so that we can try and join them only to return a tiny subset of it. – gilliduck Jul 18 '18 at 21:25
4

Main responsibility of Repository pattern to abstract actual database from domain codebase.
When you have one repository per entity you will leak database implementation details back to the domain layer.

Instead have domain based abstractions, for example

public interface ISalesOrderRepository
{
    IEnumerable<SalesOrderBasicDto> GetAll();
    SalesOrderBasicDto GetById(Guid orderId);
    SalesOrderWithLinesDto GetWithLinesById(Guid orderId);
} 

Then in database access project you can implement this repository in the most efficient way current database framework will allow.

public class SqlServerSalesOrderRepository : ISalesOrderRepository
{
    private readonly ContextFactory _contextFactory;

    public SqlServerSalesOrderRepository(ContextFactory contextFactory)
    {
        _contextFactory = contextFactory;
    }

    publc SalesOrderWithLinesDto GetWithLinesById(Guid orderId)
    {
        // Here you can use joins to combine order and related order lines
        using (var context = _contextFactory.Create<SalesContext>())
        {
            return context.SalesOrders
                          .Include(o => o.SalesOrderLines)
                          .Where(o => o.Id == orderId)
                          .Select(o => o.ToDto())
                          .Single();
        }
    }
}

So, instead of mirroring database structure in your repository, have abstractions which fit domain needs and then implement those abstractions by effectively using database features.

  • Interested to hear why this was downvoted? So, instead of mirroring database structure in your repository, have abstractions which fit domain needs. Spot on, +1. – JᴀʏMᴇᴇ Jul 19 '18 at 8:35
  • How do you handle needing both entity-specific operations (e.g. an MVC backend that allows CRUD on tables, one entity at a time, which would require entity-specific repositories) and the abstracted operations (which operate on more than one entity type, thus using your absracted repository suggestions)? Create both types of repositories? How do you then avoid WET when Foo has a certain IsBar logic that needs to be used both in the specific FooRepository and the abstracted FooAndBazRepository? – Flater Jul 19 '18 at 12:30
  • My niggle with this approach is that your repo is returning DTOs, of which you have two for the same thing, rather than Domain Objects. Its easy to inflate into SaleOrdersWithLinesAndTransactions, SaleOrdersWithCustomerButNoLines etc etc – Ewan Jul 19 '18 at 12:47
  • @Flater, I will have separated repository classes for CRUD operation and complex data queries. Repository executes only operations which database is responsible for: Read data and Save data. So IsBar logic will be outside of Repository. – Fabio Jul 19 '18 at 12:47
  • @Ewan, this was just an example with poor naming, but I prefer to explicitly "tell" what I am expecting from particular method. For example domain object order with empty collection of lines, can lead in wrong assumptions that order doesn't have lines. And loading lines only to satisfy domain object when I need only basic info about order looks resource wasting. – Fabio Jul 19 '18 at 12:54
1

What also doesn't seem right is doing the join inside the repo and then returning something that isn't one of the entities.

This is the essential imperfection of UOW-less repositories. Repositories are scoped to a single entity type. Anything that's part of the repository (i.e. unique to a single respository object) is therefore also inherently scoped to a single entity type.

This is mostly a stylistic and theoretical argument. Repositories are perfectly capable of returning more than one type, but it feels dirty to do so when the repository was created in essence to handle one given entity type.

In my opinion, this is nothing more than developer error. You've implemented a system that works on a technical level, but has considerable performance issues. Uow-less repositories are built under the assumption that database interactions are limited to object-based get/set methods; and that any data stitching or operations (e.g. a group by + count) is done in memory.

While that works on a technical level, it fails on a performance level. The intended pattern simply does not fit the needed execution.

Our typical design is to have a context wrapped inside an Repository which is Dependency Injected into a UOW class. This way we can have a Unit Test that mocks the Repo returning fake DB results.

A unit of work tackles this exact issue. It inverts the order of operations. Instead of many repositories with each their own context, you get one context with many repositories.

The short answer here is that if you want your concerns to be solved without flaws or half-assed workarounds, then you need to use a unit of work. End of story.

However, reality doesn't always agree with us. I am currently faced with a project where I simply cannot change the team's assertion that a unit of work is not worth the time to implement. Try as you might, you might be stuck in a similar situation.

So what do you do then?


In my experience as a developer, I've seen other approaches to try and tackle this issue. They are, in my opinion, inferior to a unit of work, but they are sometimes easier and good enough for a small-size application. I just want to point out the notable ones and why they were/n't good.

1. Also have repositories that scope to more than just one entity type.

For example, if a Person has many Hats and many Cats, then you'd expect 3 separate repositories. However, if you only ever access the hats and cats as part of a person (never by themselves), the Hat and Cat entities are not on equal footing with the Person entity. They are a "subordinate" entity that is effectively used as a property which simple happens to be IEnumerable but otherwise works exactly like a property.

In such a case, I've seen repositories like PersonDetailRepository created, effectively telling you that this repository is in scope of the Person entity and all of its subordinate entities.

These can coexist with entity-scoped repositories. For example, you might have an admin backend which allows users to create entities in a table; and you might have an end user website where they can look at a person+cat+hat data object.

The issue with this approach is that you end up duplicating a lot of logic between the PersonRepository and the PersonDetailRepository, and it doesn't even cover use cases for joining "main" (as opposed to "subordinate") entities together.

2. Require that a repository returns a list of its main entity with possible nav props.

In other words:

  • When you get a list of cats (including their owner), that's a CatRepository method.
  • When you get a list of people (including their cats), that's a PersonRepository method.

In this approach, it's okay to use more than one entity type in a repository, as long as the main return type of the method matches the entity type of the repository itself.

I've used this with better success than approach 1. This creates a consistent pattern, allows for you to join data to your heart's content, but still mostly streamline the methods in a way that every method only has one logical place to exist (instead of putting it in any repository for any of the used entity types in your query).

Is it perfect? No. You still don't have transactional safety when updating many entities of different types. But for data retrieval, which is where most of your joining logic tends to take place, this does give you a way to keep the separation sensical, even if it's not theoretically perfect.

1

It's a bit confusing because your pseudo code example suggests a generic repo per entity, but your real-life example has a repo for a DB (or at least a set of related entities).

However, I would say there is nothing particularly wrong with your real-life example.

Your Repository returns Domain Entities which map to tables and have methods which presumably use fast queries rather than exposing a IQueryable and leaving it to the user to discover which queries are slow.

Your AccountingInteraction class is presumably part of an application and assembles a CheckDetail ViewModel containing various Domain Entities.

As long as the exposed repository methods are performant the assembly of the ViewModel should also be.

The ViewModel is correctly separated from the Domain and Data Layers.

If I had any criticism I would say that CheckDetail would be improved by simply including whole Domain Entities and the extra filters could be moved to the repository for performance. This depends on the details of your case though, perhaps a very specific ViewModel is easier and the where clause logic is a specific business case you want to avoid putting in the data layer

eg

//include the extra parameters for your where clause, or choose an appropriate method name.

var todaysChecks = _accountingRepo.GetChecksRequiringPaymentByDate(checkDate).ToList();

//don't bother with selecting individual properties from the Domain Entities. Let the View decide what to show
return (from c in todaysChecks
                select new CheckDetail
                {
                    Vendor = vendors.FirstOrDefault(v=>c.APCODE == v.VENDCODE)
                    Check =  c,
                    CheckDetails = apCheckDetails.FirstOrDefault(d=>c.CheckId == d.CheckId),
                    //slightly awkward in the linq-sql syntax, but you get the idea
                    Transactions = tranDetails.Where(t=> t.InvTranNo == apCheckDetails.FirstOrDefault(d=>c.CheckId == d.CheckId).TranNo
                }).ToList();

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.