Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

We have a system with a non standard database solution. All trips to the DB are rather expensive. We cannot use entity framework.

Currently our lazy loading is on an entity by entity basis. So if I have a Customer and access their Orders object it only loads the Orders for that customer.

Something like so

//DAL
public List<Entities.Customer> GetCustomersByIds(IEnumerable<int> ids)
{
    var customers = db.GetCustomersByIds(ids);
    foreach(var customer in customers)
    {
        customer.Orders = new ResetLazy<List<Entities.Order>>(() => 
            db.GetOrdersByCustomerId(c.Id));
    }
    return customers;
}

ResetLazy taken from here http://stackoverflow.com/a/6255398/102526

Ideally if we had a collection of Customers and accessed an Orders collection on one of the customers it would load all the Orders for all the Customers. (If not in a collection it would just load it's own orders)

//DAL
public List<Entities.Customer> GetCustomersByIds(IEnumerable<int> ids)
{
    var customers = db.GetCustomersByIds(ids);
    foreach(var customer in customers)
    {
        customer.Orders = new ResetLazy<List<Entities.Order>>(() => 
            GetOrdersByCustomersLazy(customers, customer.Id));
    }
    return customers;
}

protected List<Entities.Order> GetOrdersByCustomersLazy(
    List<Entities.Customer> customers, 
    int customerId)
{
    var orders = db.GetOrdersByCustomerIds(
        customers.Select(customer => customer.Id).AsEnumerable());
    foreach(var customer in customers)
    {
        customer.Orders = new ResetLazy<List<Entities.Order>>(() => {
            customer.Orders = new ResetLazy<List<Entities.Order>>(() => 
                GetOrdersByCustomersLazy(customers, customer.Id));
            return orders.Where(order => order.CustomerId == customer.Id).ToList();
        });
    }
    return customers.FirstOrDefault(customer => customer.Id == customerId).Orders.Value;
}

This seems to work well, but it is too complex and not generic.

share|improve this question

You code seems to not doing what it is supposed to do.

Assume the following implementation (using the ResetLazy)

Customer and Order

namespace Entities
{
    public class Customer
    {
        public int Id { get; set; }
        public ResetLazy<List<Entities.Order>> Orders {get;set;}
    }
    public class Order
    {
        public int Id { get; set; }
        public int CustomerId { get; set; }
    }
}

Simulated database

public class TheDatabase
{
    public TheDatabase()
    {
        customers.Add(new Entities.Customer { Id = 1 });
        orders.Add(new Entities.Order { Id = 1, CustomerId = 1 });
        orders.Add(new Entities.Order { Id = 2, CustomerId = 1 });
        orders.Add(new Entities.Order { Id = 3, CustomerId = 1 });

        customers.Add(new Entities.Customer { Id = 2 });

        customers.Add(new Entities.Customer { Id = 3 });
        orders.Add(new Entities.Order { Id = 4, CustomerId = 3 });
        orders.Add(new Entities.Order { Id = 5, CustomerId = 3 });
    }
    private List<Entities.Customer> customers = new List<Entities.Customer>();
    private List<Entities.Order> orders = new List<Entities.Order>();
    public List<Entities.Customer> GetCustomersByIds(IEnumerable<int> ids)
    {
        return customers.Where(c => ids.Contains(c.Id)).ToList();
    }
    public IEnumerable<Entities.Order> GetOrdersByCustomerIds(IEnumerable<int> ids)
    {
        return orders.Where(c => IsContained(c.CustomerId, ids));
    }
    private bool IsContained(int id, IEnumerable<int> ids)
    {
        foreach (int currentId in ids)
        {
            if (id == currentId) { return true; }
        }
        return false;
    }
}  

And the class containing your methods

class Program
{

    static void Main(string[] args)
    {
        Program p = new Program();
        IEnumerable<int> ids = new List<int>() { 1, 2, 3 };
        List<Entities.Customer> customers = p.GetCustomersByIds(ids);

        int customerId = 1;
        List<Entities.Order> orders = new List<Entities.Order>(customers.Where(c => c.Id == customerId).First().Orders.Value);
        System.Diagnostics.Debug.Assert(orders[0].CustomerId == customerId);
    }

    TheDatabase db;
    private Program()
    {
        db = new TheDatabase();
    }

    public List<Entities.Customer> GetCustomersByIds(IEnumerable<int> ids)
    {
        var customers = db.GetCustomersByIds(ids);
        foreach (var customer in customers)
        {
            customer.Orders = new ResetLazy<List<Entities.Order>>(() =>
                GetOrdersByCustomersLazy(customers, customer.Id));
        }
        return customers;
    }

    protected List<Entities.Order> GetOrdersByCustomersLazy(
        List<Entities.Customer> customers,
        int customerId)
    {
        var orders = db.GetOrdersByCustomerIds(
            customers.Select(customer => customer.Id).AsEnumerable());
        foreach (var customer in customers)
        {
            customer.Orders = new ResetLazy<List<Entities.Order>>(() =>
            {
                customer.Orders = new ResetLazy<List<Entities.Order>>(() =>
                    GetOrdersByCustomersLazy(customers, customer.Id));
                return orders.Where(order => order.CustomerId == customer.Id).ToList();
            });
        }
        return customers.FirstOrDefault(customer => customer.Id == customerId).Orders.Value;
    }
}

The Debug.Assert() method signals the wrong id.

Fixing

By adding a setter to ResetLazy.Value like

set
{

    if (mode != LazyThreadSafetyMode.None)
    {
        lock (syncLock)
        {
            this.box = new Box(value);
        }
    }
    else
    {
        this.box = new Box(value);
    }
}

and changing the GetOrdersByCustomersLazy() method to

protected List<Entities.Order> GetOrdersByCustomersLazy(
    List<Entities.Customer> customers,
    int customerId)
{
    var orders = db.GetOrdersByCustomerIds(
        customers.Select(customer => customer.Id).AsEnumerable());
    foreach (var customer in customers)
    {
        customer.Orders.Value = orders.Where(o => o.CustomerId == customer.Id).ToList();
    }
    return customers.FirstOrDefault(customer => customer.Id == customerId).Orders.Value;
}  

The following runs as expected

Program p = new Program();
IEnumerable<int> ids = new List<int>() { 1, 2, 3 };
List<Entities.Customer> customers = p.GetCustomersByIds(ids);

int customerId = 1;
List<Entities.Order> orders = new List<Entities.Order>(customers.Where(c => c.Id == customerId).First().Orders.Value);
System.Diagnostics.Debug.Assert(orders.Count==3 && orders[0].CustomerId == customerId);

customerId = 2;
orders = new List<Entities.Order>(customers.Where(c => c.Id == customerId).First().Orders.Value);
System.Diagnostics.Debug.Assert(orders.Count==0);

customerId = 3;
orders = new List<Entities.Order>(customers.Where(c => c.Id == customerId).First().Orders.Value);
System.Diagnostics.Debug.Assert(orders.Count == 2 && orders[0].CustomerId == customerId);

You shouldn't shorten names of variables. The advantage of typing less is reduced by the lack of readability.


A call to the linq Select() method returns an IEnumerable so a call to AsEnumerable() is not needed.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.