Filtering values for GridViewItems

Question

I have this function in my application that I need to simplify in order to improve the code quality:

void FilterValues()
    {
        List<SecondaryStockRoomDefenition> GridViewItems = new List<SecondaryStockRoomDefenition>();
        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 == cboStockRoomCategory.SelectedValue.ToString().Trim()).ToList();
        }

        if (txtStockRoomType.Text != "")
        {
            GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
                GridViewItems[index].DeletedSecondaryStockRoom == false
                && GridViewItems[index].SecondaryStockroomType == txtStockRoomType.Text.Trim()).ToList();
        }
        if (cboExpectedUserName.SelectedValue.ToString().Trim() != "0")
        {
            GridViewItems = GridViewItems.Where((SecondaryStockRoomDefenition, index) =>
                GridViewItems[index].DeletedSecondaryStockRoom == false
                && GridViewItems[index].ExpectedUserNameForCommissionEntry == cboExpectedUserName.SelectedValue.ToString().Trim()).ToList();
        }


        dgvCompanyProfileDetails.DataSource = null;
        dgvCompanyProfileDetails.Rows.Clear();
        dgvCompanyProfileDetails.DataSource = GridViewItems;
    }

Answer 1

Just a few things from me:

1. 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

2. 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.

3. If gridViewItems is going to be assigned to stockrooms immediately, why not just do straight off the cuff?

   var gridViewItems = StockRooms.ToList();
   

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