patterncsharpMinor
Displaying information about an order when selection changes
Viewed 0 times
orderselectiondisplayingchangesaboutwheninformation
Problem
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:
below is the code calling the data:
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();
}
#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 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 = new List();
OrderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();
if(OrderInfo.Count new { x.ItemID, x.Qty, x.Item.FullName, x.Item.Brand, x.Item.Color }).ToList();
}
}Solution
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:
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
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
Braces
Always add your braces to your
Inline declaration
This snippet
is an elaborate way of writing
Notice also how I made
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 GetOrderById(int id);
}
class OrderRepository : IOrderRepository {
public IEnumerable 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 = new List();
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!Code Snippets
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));
}
}List<OrderInfo> OrderInfo = new List<OrderInfo>();
OrderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();var orderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();Context
StackExchange Code Review Q#47890, answer score: 5
Revisions (0)
No revisions yet.