3

I am using Code First to automatically generate my database, and this works perfectly, generating an Orders table and an OrderLines table as expected when I add some test data.

I have the following Order class:

public class Order
{
    public int OrderID { get; set; }

    public void AddItem(string productCode, int quantity)
    {
        var existingLine = OrderLines.FirstOrDefault(x => x.ProductOption.ProductCode == item.ProductCode);

        if (existingLine == null)
            OrderLines.Add(new OrderLine { ProductOption = item, Quantity = quantity });
        else
            existingLine.Quantity += quantity;
    }

    public void RemoveItem(string productCode)
    {
        OrderLines.Remove(OrderLines.Where(x => x.ProductOption.ProductCode == productCode).FirstOrDefault());
    }

    public virtual ICollection<OrderLine> OrderLines { get; set; }

    public Order()
    {
        OrderLines = new List<OrderLine>();
    }
}

What I really want is to encapsulate the OrderLines collection, making it impossible for consumers of the class to directly add and remove items to/from it (using the Add / Remove methods of ICollection) and instead forcing them to use my custom AddItem and RemoveItem methods.

Normally I could just make the collection private, but I can't do that because it needs to be virtual for EF to correctly create the OrderLines table/foreign keys.

This answer seems to suggest that making the property internal would do the trick, but I tried, and in that case no OrderLines table is created.

Is there any way that this can be accomplished, or should I have designed this differently somehow? Any help much appreciated!

Update

After a bit more searching, I found this question which is rather more clearly stated than mine; however, it's still unanswered. The poster does link to this post which seems to suggest it can't really be done in the way I'm thinking of, but does anyone have any more up-to-date information?

Community
  • 1
  • 1
Mark Bell
  • 28,985
  • 26
  • 118
  • 145

3 Answers3

1

I don't know if it's possible to do what you are asking or not, but I'm not sure it's the best design. The problem that I am seeing is you are firmly integrating your business logic into your business entities, and I think this will turn into confusion down the road.

Take the following scenario under consideration. Say you have a new requirement where you want users to be able to remove all items from an order. The only way to do it with your entity is to create a new RemoveAllItems() method to your Order class which does that. Now say you have a new requirement to Remove all items from an order that are in a specific category. That then means that you have to add yet another method.

This causes really bloated classes, and there is one major issue you will come up with. If you (or another developer) want to look at an entity and determine it's data structure, you can't at a glance because it's so intertwined with business logic.

What I would suggest is that you keep your entities as pure data structures, keeping all their relationships public. Then you need to create a service layer, which can consist of small or big classes (however you want to organize them) that actually perform the business functions. So for example, you can have a OrderItemService class, which has methods for adding, editing, and removing items from an order. All your business logic is performed in this class, and you just have to enforce that only service classes are allowed to interact with db entities.

Now, if you are looking for how a particular business process is performed, you know to look in the service layer classes, and if you want to look at how a data structure or entity is organized, you look at the entity. This keeps everything clean and very mantainable.

KallDrexx
  • 27,229
  • 33
  • 143
  • 254
  • Thanks for that, this seems like a sensible idea, and in fact I was looking at implementing a service layer anyway. I think I just got carried away with trying to keep everything related to `Orders` in one place. – Mark Bell Feb 14 '11 at 19:41
  • Even though this doesn't answer the question directly, I'm accepting your answer because it was the solution I went with—besides which, I can't find any way of doing this that doesn't feel like a bit of a hack. – Mark Bell Feb 15 '11 at 17:49
  • 13
    If you're going for DDD, then entities as pure data structures is an anti pattern known as an Anemic Domain. Careful... – Joshua Ramirez May 16 '11 at 22:53
  • 3
    Business entities have nothing to do with the business unless they contain business logic. You end up with meaningless data transfer objects and lose a lot OO power. I disagree with the accepted answer. – btt Jul 02 '11 at 04:20
  • I disagree with the answer (if the object gets too "bloated"/does too many things then you need to split it up), but agree somewhat with the example. `RemoveAllItems()` is not a `Order` responsibility. Moving it to something called `OrderItemService` is also wrong, though. It really belongs in a [repository-pattern](http://martinfowler.com/eaaCatalog/repository.html) class (maybe as an extension method on [`IDbSet`](http://msdn.microsoft.com/en-us/library/gg679233.aspx), since that is EF's repository implementation). – Merlyn Morgan-Graham Sep 19 '12 at 05:46
  • Putting validation for the collection inside a separate service class feels dirty. Take for example a Vehicle class. Let's assume the vehicle can't have more than 4 wheels. Because the service has flexibility to create any required accessor/setters for the collection, the collection on the Vehicle object has to be fully exposed. This means that poorly written code in the service class could add a 5th or more wheels to the collection. In my mind, the logic to check that there are only 4 wheels belongs with the object, and not inside multiple service class methods. – Chris Jan 13 '13 at 05:44
0

I'm far from an expert on code first and I haven't tried the following but is it possible to use the ReadOnlyCollectionBase and create a read only list similar to this MSDN article?

lancscoder
  • 8,658
  • 8
  • 48
  • 68
  • I don't think many people are experts on it at the moment! The point isn't that I _can't_ make the collection read-only, it's that if I do, then EF doesn't create the table representing its children. – Mark Bell Feb 14 '11 at 19:31
0

Well what you can do is set your collection as private and make the relationship using fluent API in the OnModelCreating, as shown below, I don't know if this will work, just make a try:

public class YourContext : DbContext
{
   public DbSet<Order> Orders { get; set; }
   public DbSet<OrderLine> OrderLines { get; set; }

   protected override void OnModelCreating(ModelBuilder modelBuilder)
   {
       modelBuilder.Entity<Order>() 
           .HasMany(o => o.OrderLines) 
           .WithRequired(l => l.OrderId) 
           .HasForeignKey(l => l.OrderId);

   }
}

This will make your OrderLines as readonly:

public class YourContext : DbContext
{
   public DbSet<Order> Orders { get; set; }

   public DbSet<OrderLine> OrderLines 
   { 
      get { return set<OrderLine>(); }
   }
}

I hope this can help you, please take a look a this blog post: EF Feature CTP5: Fluent API Samples

Ramón García-Pérez
  • 1,880
  • 2
  • 20
  • 25