HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Filtering values for GridViewItems

Submitted by: @import:stackexchange-codereview··
0
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

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.

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.