Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

When to expose an IEnumerable instead of an ICollection?

public class Order
{
 public int Id {get;set;}
 [DisplayName("User")]
 public long UserId { get; set; }
 [ForeignKey("UserId")]
 public virtual User User { get; set; }
 public decimal Amount { get; set; }
}

With IEnumurable

public class User
{
 public int Id{get;set;}
 public virtual IEnumerable<Order> Orders { get; set; } 
}

public User GetWithOrders()
{
 var myUser=UserRepository.GetByEmail("[email protected]");
 myUser.Orders=OrderRepository.GetByUserId(myUser.Id);
 return myUser;
}

With ICollection

public class User
{
 public int Id{get;set;}
 public virtual ICollection<Order> Orders { get; set; } 
}

public User GetWithOrders()
{
 var myUser=UserRepository.GetByEmail("[email protected]");
 return myUser;
}

I don't have lazy loading using IEnumerable for a navigation property. Therefore, I have to get the orders for this user with another query.

I have navigation with ICollection. So I can reach orders from user. This seems cool. But then I can add new orders to the user in the Controller without using service or repository.

It's kind of manipulating data on controller level. Is this anti-pattern?

like image 433
Erkan Demirel Avatar asked Sep 27 '22 21:09

Erkan Demirel


2 Answers

But [with ICollection] I can add new order in Controller without using service or repository.

You mean you can do this (assuming there's a viewmodel for adding an order to a user and a SaveChanges() somewhere):

public class UserController
{
    public ActionResult AddUserOrder(AddUserOrderModel addOrder)
    {
        User user = User.GetByEmail(addOrder.UserEmail);        
        user.Orders.Add(addOrder.Order);
        User.SaveChanges();
    }
}

And especially that you can do user.Orders.Add(...), then that's a side effect of exposing entity types from your service or repository layer.

If you want to avoid that, you'd have to define and expose a business object containing the members you want to expose:

public class UserBLL
{
    public int Id { get; private set; }
    public IEnumerable<Order> Orders { get { return _orders.AsEnumerable(); } }
    private IEnumerable<Order> _orders;

    public UserBLL(User user)
    {
        Id = user.Id;
        _orders = user.Orders;          
    }

    public void AddOrder(Order order)
    {
        _orders.Add(order);
    }
}
like image 55
CodeCaster Avatar answered Oct 06 '22 18:10

CodeCaster


There's not a real choice here. ICollection is needed by EF to control some of it's aspects like binding query results and lazy loading. By using IEnumerable, you're essentially turning all this functionality off, but along with it, EF's understanding of your underlying structure. When you generate migrations, EF will not generate any requisite underlying join tables for M2M relationships, foreign keys on related tables, etc.

Long and short, use ICollection. While you're correct that this allows you to add items by simply adding them to the collection on a related entity, sans-DAL, they still can't be saved without access to the context. If you've set up your DAL correctly, that's only available through the DAL, itself, so you still have to pass the entity back into your DAL pipeline to perpetuate any of these changes. In other words, don't worry about it.

like image 28
Chris Pratt Avatar answered Oct 06 '22 19:10

Chris Pratt