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:

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; }

public class Item

    public Item()
        this.Orders = new List<OrderInfo>();
    #region Strings
    public string Color { get; set; }
    public string FullName { get; set; }
    public string Type { get; set; }
    public string PrimaryMachine { get; set; }
    public string Alias { get; set; }
    public string Brand { get; set; }


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

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

    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!");

            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();
  • 1
    \$\begingroup\$ I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ Mar 1, 2021 at 21:12

1 Answer 1



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.


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.


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!

  • \$\begingroup\$ Do you have any more links? I have been trying to implement the Repository system, and keepo having issues like from that link when I try to use the IQueryable<Order> AllIncluding(params Expression<Func<Order, object>>[] includeProperties); method I get an error query = query.Include(includeProperty); that Include is not part of Context.Order. Any additional links would be great \$\endgroup\$ Apr 23, 2014 at 20:01
  • \$\begingroup\$ Google yields many results, all of them which look pretty good. If you need assistance with not-working code, don't forget to visit Stack Overflow! \$\endgroup\$ Apr 23, 2014 at 20:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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