Displaying information about an order when selection changes

Question

I feel like just because the below works doesn't mean it is correct, I want to improve it but I can't really figure out how. Besides the fact it is down right ugly I feel the performance could be increased a lot also.

Am I even using the Entity Framework and/or LINQ the way it is supposed to be used?

So I have these classes:

[Table("OrderInfo")]
public class OrderInfo
{
    public long ID {get; set;}
    public long OrderID {get; set;}
    public virtual Order Order { get; set; }
    public long ItemID {get; set;}
    public double Qty { get; set; }
    public virtual Item Item { get; set; }
}

[Table("Items")]
public class Item
{

    public Item()
    {
        this.Orders = new List<OrderInfo>();
    }
    #region Strings
    public string Color { get; set; }
    public string FullName { get; set; }
    [Column(@"Sheet/Roll")]
    public string Type { get; set; }
    public string PrimaryMachine { get; set; }
    public string Alias { get; set; }
    public string Brand { get; set; }

    #endregion

    #region Long
    public long ID { get; set; }
    public long? Weight { get; set; }
    #endregion

    #region Doubles
    public double? Size1 { get; set; }
    public double? Size2 { get; set; }
    public double? Size3 { get; set; }
    #endregion.

    public virtual ICollection<OrderInfo> Orders { get; set; }
}

below is the code calling the data:

    private void dgvOrders_SelectionChanged(object sender, EventArgs e)
    {
        DataGridView dgv = (DataGridView)sender;

        if (dgv.SelectedRows.Count > 0)
        {
            if (dgvOrderItems.DataSource != null)
                dgvOrderItems.DataSource = null;

            int ID = Convert.ToInt32(dgv["ID", dgv.SelectedRows[0].Index].Value);
            List<OrderInfo> OrderInfo = new List<OrderInfo>();
            OrderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();

             if(OrderInfo.Count <= 0)
             {
                MessageBox.Show("No Info Found For This Order!");
                ClearForm();
                return;
             }

            lblPO.Text = "P.O. # " + OrderInfo[0].ID.ToString();
            lblRequestedBy.Text = "Requested By: " + OrderInfo[0].Order.RequestedBy;
            lblOrderDate.Text = "Ordered On: " + OrderInfo[0].Order.Date.ToShortDateString();

            dgvOrderItems.DataSource = OrderInfo.Select(x => new { x.ItemID, x.Qty, x.Item.FullName, x.Item.Brand, x.Item.Color }).ToList();
        }
    }

Answer 1

Architecture

One thing that jumps forward is that you're going straight from the UI to your datasource. In a three-layer design (UI, Model, Persistence) or MVC (Model, View, Controller) there is always a layer between the UI and your datasource.

A samplesetup would be this:

interface IOrderRepository {
    IEnumerable<OrderInfo> GetOrderById(int id);
}

class OrderRepository : IOrderRepository {
    public IEnumerable<OrderInfo> GetOrderById(int id){
        using(var context = new MyDataContext()){
            return context.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();
        }
    }
}

class UIController {
    private IOrderRepository _orderRepository;
    
    public void OnOrdersSelectionChanged(DataGridView view){
        _orderRepository.GetOrderById(Convert.ToInt32(dgv["ID", dgv.SelectedRows[0].Index].Value));
    }
}

Now your view is coupled loose from your datasource through the intermediate repository. You might be interested in reading up on the repository pattern, for example here.

Resources

I don't see a using statement in your code so I assume that your DataContext is held as a private field. This context class is explicitly made to be disposed after using it once so you should use it as such.

In the MSDN blog I linked just before the repository is disposed instead which is also an option although I have found that wrapping all calls to the DataContext is more often used.

Braces

Always add your braces to your if statements, even when it's just one line. It's simply too easy to make mistakes.

Inline declaration

This snippet

List<OrderInfo> OrderInfo = new List<OrderInfo>();
OrderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();

is an elaborate way of writing

var orderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();

Notice also how I made orderInfo lowercase. Keep naming conventions in mind!