Domanda

I have following fully functional method that returns a list depending on the input-parameters (it returns everything if no valid input was found):

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
    List<PriceRow> lstFoundPriceRows = _lstABSPriceRows; //_lstABSPriceRows is the source list

    if ((dekorNr != null) && !dekorNr.Trim().Equals(String.Empty))
        lstFoundPriceRows = lstFoundPriceRows.FindAll(p => p.Artikelnummer.Contains(dekorNr));

    if ((bezeichnung != null) && !bezeichnung.Trim().Equals(String.Empty))
        lstFoundPriceRows = lstFoundPriceRows.FindAll(p => p.Bezeichnung.Contains(bezeichnung));

    if ((hersteller != null) && !hersteller.Trim().Equals(String.Empty))
        lstFoundPriceRows = lstFoundPriceRows.FindAll(p => p.Lieferant.Contains(hersteller));

    return lstFoundPriceRows;
}

The three parameters can be null or String.Empty and should only be used for filtering the source-list if they are not null or String.Empty.

As I said the code works fine but I'm not happy with it ;). It seems to be too complicated. Is there any way to create only one elegant dynamic linq-statement?

È stato utile?

Soluzione

First you can simplify the if conditions:

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
    List<PriceRow> lstFoundPriceRows = _lstABSPriceRows; //_lstABSPriceRows is the source list

    if (!string.IsNullOrWhitespace(dekorNr))
        lstFoundPriceRows = lstFoundPriceRows.FindAll(p => p.Artikelnummer.Contains(dekorNr));

    if (!string.IsNullOrWhitespace(bezeichnung))
        lstFoundPriceRows = lstFoundPriceRows.FindAll(p => p.Bezeichnung.Contains(bezeichnung));

    if (!string.IsNullOrWhitespace(hersteller))
        lstFoundPriceRows = lstFoundPriceRows.FindAll(p => p.Lieferant.Contains(hersteller));

    return lstFoundPriceRows;
}

Secondly you can use a where clause which does not actually perform a scan on the list (you are preforming 3 scans right now):

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
    IEnumerable<PriceRow> lstFoundPriceRows = _lstABSPriceRows; //_lstABSPriceRows is the source list

    if (!string.IsNullOrWhitespace(dekorNr))
        lstFoundPriceRows = lstFoundPriceRows.Where(p => p.Artikelnummer.Contains(dekorNr));

    if (!string.IsNullOrWhitespace(bezeichnung))
        lstFoundPriceRows = lstFoundPriceRows.Where(p => p.Bezeichnung.Contains(bezeichnung));

    if (!string.IsNullOrWhitespace(hersteller))
        lstFoundPriceRows = lstFoundPriceRows.Where(p => p.Lieferant.Contains(hersteller));

    return lstFoundPriceRows.ToList();
}

Since now you are doing one scan anyways you can move the conditions in the Where predicates:

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
    IEnumerable<PriceRow> lstFoundPriceRows = _lstABSPriceRows; //_lstABSPriceRows is the source list

    lstFoundPriceRows = lstFoundPriceRows.Where(p => string.IsNullOrWhitespace(dekorNr) || p.Artikelnummer.Contains(dekorNr));

    lstFoundPriceRows = lstFoundPriceRows.Where(p => string.IsNullOrWhitespace(bezeichnung) || p.Bezeichnung.Contains(bezeichnung));

    lstFoundPriceRows = lstFoundPriceRows.Where(p => string.IsNullOrWhitespace(hersteller) || p.Lieferant.Contains(hersteller));

    return lstFoundPriceRows.ToList();
}

Since you have no more ifs, you can combine the statements into one.

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
    return _lstABSPriceRows
           .Where(p => string.IsNullOrWhitespace(dekorNr) || p.Artikelnummer.Contains(dekorNr))
           .Where(p => string.IsNullOrWhitespace(bezeichnung) || p.Bezeichnung.Contains(bezeichnung))
           .Where(p => string.IsNullOrWhitespace(hersteller) || p.Lieferant.Contains(hersteller))
           .ToList();
}

Finally, we can group all the Where predicates into one (thanks @Kris):

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
    return _lstABSPriceRows
           .Where(p => (string.IsNullOrWhitespace(dekorNr) || p.Artikelnummer.Contains(dekorNr)) && 
                       (string.IsNullOrWhitespace(bezeichnung) || p.Bezeichnung.Contains(bezeichnung)) &&
                       (string.IsNullOrWhitespace(hersteller) || p.Lieferant.Contains(hersteller)))
           .ToList();
}

You can create a simpler (and 3.5 compatible) of IsNullOrWhitespace by using an extension method like so:

public static bool IsNullOrWhitespace(this string s) 
{
   return (s == null || string.IsNullOrEmpty(s.Trim()));
}

And with it the expression becomes even simpler:

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
    return _lstABSPriceRows
           .Where(p => (dekorNr.IsNullOrWhitespace() || p.Artikelnummer.Contains(dekorNr)) && 
                       (bezeichnung.IsNullOrWhitespace() || p.Bezeichnung.Contains(bezeichnung)) &&
                       (hersteller.IsNullOrWhitespace() || p.Lieferant.Contains(hersteller)))
           .ToList();
}

Altri suggerimenti

The code runs fine but is too complicated. For instance; what happens if you input a deKorNr and a hersteller? I do not know if you could fix that with one epic linq statement and make it less complicated...

A better solution would be to create three separate functions which all have their own logic. Then it would not require your original question in the first place.

public List<PriceRow> GetABSPriceRows(string dekorNr, string bezeichnung, string hersteller)
{
  List<PriceRow> lstFoundPriceRows = _lstABSPriceRows; //_lstABSPriceRows is the source list

    if (!string.IsNullOrWhitespace(dekorNr) || !string.IsNullOrWhitespace(bezeichnung)|| !string.IsNullOrWhitespace(hersteller) )
      {  
        lstFoundPriceRows = lstFoundPriceRows.Where (p => p.Artikelnummer.Contains(dekorNr) ||         
          p.Bezeichnung.Contains(bezeichnung)||p.Lieferant.Contains(hersteller)).ToList();   
       }    

    else
    {
     // Your query without filters
    }
  }
Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top