2
\$\begingroup\$

I have two databases: one for a warehouse, second for the office (The db design is out of the scope of this question). There are MANY cross-database funtions. One of those functions is IsPurchaseOrderClosed. I need data from TWO databases to determine this.

I was trying to build a DAL using UnitOfWork and Repository pattern (I always created UoW and Repository classes and interfaces. I knew DbContext already implements both patterns.. but I wanted to abstract EF layer). Anyway.. in this case, I decided to remove both elements and stick with DbContext and Service layer.

Here's my solution:

public class WarehouseContext : DbContext
{
    public WarehouseContext()
        : base("name=WarehouseContext")
    {
        this.Configuration.LazyLoadingEnabled = false;
    }

    public virtual DbSet<PurchaseOrder> PurchaseOrders { get; set; }
    public virtual DbSet<Inventory> Inventories { get; set; }
    public virtual DbSet<Orders> Orders { get; set; }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
        Database.SetInitializer<WarehouseContext>(null);
        base.OnModelCreating(modelBuilder);
    }
}

public class OfficeContext : DbContext
{
    public OfficeContext()
        : base("name=OfficeContext")
    {
        this.Configuration.LazyLoadingEnabled = false;
    }

    public virtual DbSet<Invoice> Invoices { get; set; }
    public virtual DbSet<Supplier> Suppliers { get; set; }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
        Database.SetInitializer<OfficeContext>(null);
        base.OnModelCreating(modelBuilder);
    }
}


public class ContextFactory : IContextFactory
{
    public WarehouseContext CreateWarehouseContext()
    {
        return new WarehouseContext();
    }

    public OfficeContext CreateOfficeContext()
    {
        return new OfficeContext();
    }
}



public class PurchaseOrderService
{
    IContextFactory ContextFactory { get; set; }

    public PurchaseOrderService(IContextFactory contextFactory)
    {
        ContextFactory = contextFactory;
    }

    //Service method, that interact with two databases:
    public bool IsPurchaseOrderClosed(int poId)
    {
        bool toReturn = false;

        using(var WarehouseContext = ContextFactory.CreateWarehouseContext())
        {
            PurchaseOrder purchaseOrder = WarehouseContext.PurchaseOrders.Find(poId);

            if (!purchaseOrder.IsClosed)
            {
                using(var OfficeContext = ContextFactory.CreateOfficeContext())
                {
                    int invoiceId = purchaseOrder.InvoiceId;

                    Invoice invoice = OfficeContext.Invoices.Find(invoiceId);

                    if (invoice.WasSent)
                    {
                        toReturn = true;
                    }
                }
            }
            else
            {
                toReturn = true;
            }
        }
        return toReturn;
    }
}

I have two questions:

  1. Is this solution good? (By good I mean, does it meet every good-practice advice, SOLID principles etc)

  2. In IsPurchaseOrderClosed method I get tables using OfficeContext.Set<Invoice>().Find(invoiceId); to get particular Invoice object from the database. Isn't it too.. complex/not readable? I was thinking how can I make it more simple? Should I create some kinda generic Service, with all basic/CRUD methods?

  3. I have also third question: Currently ContextFactory is the part of .Data subproject (where my Contexts and Entities resides). Do you think it should be in .Business layer, which is the Service Layer (I prefer using Business or Logic words - because many people confuse it with WCF or Windows Service)?

\$\endgroup\$
  • 1
    \$\begingroup\$ What I don't understand is why do you SaveChanges if you didn't change anything? Have you posted edited code? \$\endgroup\$ – t3chb0t Jan 22 '18 at 8:09
  • \$\begingroup\$ @t3chb0t oh, I cut this code. I close the PO (I also do bunch of other stuff, but it's irrelevant). \$\endgroup\$ – Marshall Jan 22 '18 at 12:12
  • 2
    \$\begingroup\$ It is not irrelevant. Posting hypthetical/stub code is off-topic. I have to give you a close-vote for that because I'm not going to review an incomlete or edited method. It's too likely that you are going to say "oh, I already have that so it doesn't help". It'd happened too often already. \$\endgroup\$ – t3chb0t Jan 22 '18 at 12:15
  • \$\begingroup\$ @t3chb0t you're right. I just posted the original method, what I have now. I removed all "update" changes because of separation of concerns. The function returns only if PO is closed or not. Is it correct now? \$\endgroup\$ – Marshall Jan 22 '18 at 13:54
  • 1
    \$\begingroup\$ No, I didn't mean posting everything. You shouldn't :-) but just the relavant parts unchanged. By hypothetical I meant your method that wasn't real since some parts where missing. I'll retract my VTC. Thank you for fixing it. \$\endgroup\$ – t3chb0t Jan 22 '18 at 14:02

Your Answer

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

Browse other questions tagged or ask your own question.