patterncsharpMinor
Filtering values for GridViewItems
Viewed 0 times
valuesforgridviewitemsfiltering
Problem
I have this function in my application that I need to simplify in order to improve the code quality:
```
void FilterValues()
{
List GridViewItems = new List();
GridViewItems = StockRooms.ToList();
if (cboPrimaryStockRoom.SelectedValue.ToString().Trim() != "0")
{
GridViewItems =
StockRooms.Where((SecondaryStockRoomDefenition, index)
=> GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].PrimaryStockroomCode == cboPrimaryStockRoom.SelectedValue.ToString().Trim()).ToList();
}
if( txtSecondaryStockRoomCode.Text != "" )
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition,index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomCode == txtSecondaryStockRoomCode.Text.Trim()).ToList();
}
if (txtSecondaryStockRoomName.Text != "")
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomDescription.Contains(txtSecondaryStockRoomName.Text.Trim())).ToList();
}
if (txtStockRoomLocationCode.Text != "")
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomLocationCode == txtStockRoomLocationCode.Text.Trim()).ToList();
}
if (cboStockRoomCategory.SelectedValue.ToString().Trim() != "0")
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomCategory == cboStockRoomCategor
```
void FilterValues()
{
List GridViewItems = new List();
GridViewItems = StockRooms.ToList();
if (cboPrimaryStockRoom.SelectedValue.ToString().Trim() != "0")
{
GridViewItems =
StockRooms.Where((SecondaryStockRoomDefenition, index)
=> GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].PrimaryStockroomCode == cboPrimaryStockRoom.SelectedValue.ToString().Trim()).ToList();
}
if( txtSecondaryStockRoomCode.Text != "" )
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition,index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomCode == txtSecondaryStockRoomCode.Text.Trim()).ToList();
}
if (txtSecondaryStockRoomName.Text != "")
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomDescription.Contains(txtSecondaryStockRoomName.Text.Trim())).ToList();
}
if (txtStockRoomLocationCode.Text != "")
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomLocationCode == txtStockRoomLocationCode.Text.Trim()).ToList();
}
if (cboStockRoomCategory.SelectedValue.ToString().Trim() != "0")
{
GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
GridViewItems[index].DeletedSecondaryStockRoom == false
&& GridViewItems[index].SecondaryStockroomCategory == cboStockRoomCategor
Solution
Just a few things from me:
-
Check the way you name variables. Local variables by defacto standard are lower case camel. See here for more details Microsoft naming conventions.
-
I personally try to use implicit local variables where the type is obvious. This is more a personal preference but Microsoft do recommend as well. See C# coding conventions for more details.
-
If
-
I would convert this method into a bunch of smaller functions. Perhaps something like (excuse any compile options as I could not fully test this code):
-
Check the way you name variables. Local variables by defacto standard are lower case camel. See here for more details Microsoft naming conventions.
GridViewItems becomes gridViewItems-
I personally try to use implicit local variables where the type is obvious. This is more a personal preference but Microsoft do recommend as well. See C# coding conventions for more details.
-
If
gridViewItems is going to be assigned to stockrooms immediately, why not just do straight off the cuff?var gridViewItems = StockRooms.ToList();-
I would convert this method into a bunch of smaller functions. Perhaps something like (excuse any compile options as I could not fully test this code):
void FilterValues()
{
dgvCompanyProfileDetails.DataSource = null;
dgvCompanyProfileDetails.Rows.Clear();
dgvCompanyProfileDetails.DataSource = Filter(StockRooms).ToList();
}
private IEnumerable Filter(IEnumerable stockRooms)
{
// filter the stock rooms on each function call
stockRooms = FilterStockRooms(stockRooms, cboPrimaryStockRoom, p => p.PrimaryStockroomCode);
stockRooms = FilterStockRooms(stockRooms, txtSecondaryStockRoomCode.Text, p => p.SecondaryStockroomCode);
stockRooms = FilterStockRooms(stockRooms, txtSecondaryStockRoomName.Text, (p, v) => p.SecondaryStockroomDescription.Contains(v));
stockRooms = FilterStockRooms(stockRooms, txtStockRoomLocationCode.Text, p => p.SecondaryStockroomLocationCode);
// .. etc
return stockRooms;
}
private IEnumerable FilterStockRooms(IEnumerable rooms, ComboBox combo, Func filter)
{
var value = combo.SelectedValue.ToString();
return HasValue(value, "0") ? FilterStockRooms(rooms, v => filter(v).Equals(value, StringComparison.InvariantCultureIgnoreCase)) : rooms;
}
private IEnumerable FilterStockRooms(IEnumerable rooms, string text, Func filter)
{
return HasValue(text) ? FilterStockRooms(rooms, v => filter(v).Equals(text, StringComparison.InvariantCultureIgnoreCase)) : rooms;
}
private IEnumerable FilterStockRooms(IEnumerable rooms, string text, Func filter)
{
return HasValue(text) ? FilterStockRooms(rooms, v => filter(v, text)) : rooms;
}
// The question marks are because I wasn't sure of the StockRooms type
private IEnumerable FilterStockRooms(IEnumerable rooms, Func filter)
{
return rooms.Where(p => p.DeletedSecondaryStockRoom == false && filter(p));
}
private bool HasValue(string value)
{
return !string.IsNullOrWhiteSpace(value);
}
private bool HasValue(string value, string compareTo)
{
return HasValue(value) && value.Trim() != compareTo;
}Code Snippets
var gridViewItems = StockRooms.ToList();void FilterValues()
{
dgvCompanyProfileDetails.DataSource = null;
dgvCompanyProfileDetails.Rows.Clear();
dgvCompanyProfileDetails.DataSource = Filter(StockRooms).ToList();
}
private IEnumerable<SecondaryStockRoomDefenition> Filter(IEnumerable<SecondaryStockRoomDefenition> stockRooms)
{
// filter the stock rooms on each function call
stockRooms = FilterStockRooms(stockRooms, cboPrimaryStockRoom, p => p.PrimaryStockroomCode);
stockRooms = FilterStockRooms(stockRooms, txtSecondaryStockRoomCode.Text, p => p.SecondaryStockroomCode);
stockRooms = FilterStockRooms(stockRooms, txtSecondaryStockRoomName.Text, (p, v) => p.SecondaryStockroomDescription.Contains(v));
stockRooms = FilterStockRooms(stockRooms, txtStockRoomLocationCode.Text, p => p.SecondaryStockroomLocationCode);
// .. etc
return stockRooms;
}
private IEnumerable<SecondaryStockRoomDefenition> FilterStockRooms(IEnumerable<SecondaryStockRoomDefenition> rooms, ComboBox combo, Func<SecondaryStockRoomDefenition, string> filter)
{
var value = combo.SelectedValue.ToString();
return HasValue(value, "0") ? FilterStockRooms(rooms, v => filter(v).Equals(value, StringComparison.InvariantCultureIgnoreCase)) : rooms;
}
private IEnumerable<SecondaryStockRoomDefenition> FilterStockRooms(IEnumerable<SecondaryStockRoomDefenition> rooms, string text, Func<SecondaryStockRoomDefenition, string> filter)
{
return HasValue(text) ? FilterStockRooms(rooms, v => filter(v).Equals(text, StringComparison.InvariantCultureIgnoreCase)) : rooms;
}
private IEnumerable<SecondaryStockRoomDefenition> FilterStockRooms(IEnumerable<SecondaryStockRoomDefenition> rooms, string text, Func<SecondaryStockRoomDefenition, string, bool> filter)
{
return HasValue(text) ? FilterStockRooms(rooms, v => filter(v, text)) : rooms;
}
// The question marks are because I wasn't sure of the StockRooms type
private IEnumerable<SecondaryStockRoomDefenition> FilterStockRooms(IEnumerable<SecondaryStockRoomDefenition> rooms, Func<SecondaryStockRoomDefenition, bool> filter)
{
return rooms.Where(p => p.DeletedSecondaryStockRoom == false && filter(p));
}
private bool HasValue(string value)
{
return !string.IsNullOrWhiteSpace(value);
}
private bool HasValue(string value, string compareTo)
{
return HasValue(value) && value.Trim() != compareTo;
}Context
StackExchange Code Review Q#16262, answer score: 8
Revisions (0)
No revisions yet.