Функциональность фильтрации Project с использованием C #

Я просто реализую функцию фильтра для одной из моих форм. Я хотел бы услышать несколько обзоров, в которых я могу улучшить свой код или свою функциональность 🙂

Резюме

У меня есть 1 текстовое поле и 2 поля со списком (см. Изображение ниже):

Варианты категорий

Варианты статуса

Код

public MainView()
{
    InitializeComponent();
    categoryText.SelectedIndex = 0;
    statusText.SelectedIndex = 0;

    RefreshData();
}

List<ConcessionModel> concessionList = new List<ConcessionModel>();
public void RefreshData()
{
    DataAccess db = new DataAccess();
    concessionList = db.LoadConcessionLog();
    dataGrid.DataSource = concessionList;

    dataGrid.Columns["Id"].Visible = false;
    dataGrid.Columns["PersonCompletingID"].Visible = false;
}


private void searchBtn_Click(object sender, EventArgs e)
{
    dataGrid.DataSource = Filter(productText.Text, categoryText.Text, statusText.Text);
}

private List<ConcessionModel> Filter(string product, string category, string status)
{
    if (category == "All" && status != "All")
    {
        return concessionList
                .Where(x => x.Product
                .IndexOf(product, StringComparison.InvariantCultureIgnoreCase) >= 0 && x.Status
                .IndexOf(status, StringComparison.InvariantCultureIgnoreCase) >= 0)
                .ToList();
    }
    else if (category != "All" && status == "All")
    {
        return concessionList
                .Where(x => x.Product
                .IndexOf(product, StringComparison.InvariantCultureIgnoreCase) >= 0 && x.Category
                .IndexOf(category, StringComparison.InvariantCultureIgnoreCase) >= 0)
                .ToList();
    }
    else if(category == "All" && status == "All")
    {
        return concessionList
                .Where(x => x.Product
                .IndexOf(product, StringComparison.InvariantCultureIgnoreCase) >= 0).ToList();
    }
    else
    {
        return concessionList
                .Where(x => x.Product
                .IndexOf(product, StringComparison.InvariantCultureIgnoreCase) >= 0 && x.Category
                .IndexOf(category, StringComparison.InvariantCultureIgnoreCase) >= 0 && x.Status
                .IndexOf(status, StringComparison.InvariantCultureIgnoreCase) >= 0)
                .ToList();
    }
}

public void ClearConcessionFilter()
{
    productText.Clear();
    categoryText.SelectedIndex = 0;
    statusText.SelectedIndex = 0;
    dataGrid.DataSource = concessionList;
}

КонцессияМодель

public class ConcessionModel
{
    public int Id { get; set; }

    [DisplayName("Date Created")]
    public DateTime DateCreated { get; set; }

    public int PersonCompletingID { get; set; }
    public string Product { get; set; }

    [DisplayName("Batch Number")]
    public string BatchNumber { get; set; }

    [DisplayName("Quantity Affected")]
    public string QuantityAffected { get; set; }

    [DisplayName("Reason For Concession")]
    public string ReasonForConcession { get; set; }

    public string Category { get; set; }
    public string Status { get; set; }
}

Позвольте мне сосредоточиться на Filter метод.

• В этом методе много повторений, что делает ваш код
  • труднее поддерживать
    • например: замена IndexOf к Equals
    • или изменив StringComparison вариант другому
  • и многое другое, подверженное ошибкам
    • например: легко написать ошибку All при одном из происшествий.

Есть несколько полезных приемов, которые могут сделать ваш код менее подверженным ошибкам и более легким для изменения:

• Использование констант, например: const string all = "All";
• Использование одиночного возврата, например: return concessionList.Where(predicate).ToList();
• Извлечение общей логики, например:

static bool IsPresent(string source, string target)
   => source.IndexOf(target, StringComparison.InvariantCultureIgnoreCase) >= 0;

• Используя значения по умолчанию вместо резервных,
  • подобно: predicate может иметь значение по умолчанию вместо того, чтобы полагаться на окончательный else блокировать.

С этим в наших руках Filter метод можно переписать так:

private List<ConcessionModel> Filter(string product, string category, string status)
{
    const string all = "All";

    static bool IsPresent(string source, string target) 
        => source.IndexOf(target, StringComparison.InvariantCultureIgnoreCase) >= 0;

    Func<ConcessionModel, bool> predicate = model =>
        IsPresent(model.Product, product) &&
        IsPresent(model.Category, category) && 
        IsPresent(model.Status, status);

    if (category == all && status != all)
    {
        predicate = model =>
            IsPresent(model.Product, product) &&
            IsPresent(model.Status, status);
    }
    else if (category != all && status == all)
    {
        predicate = model =>
            IsPresent(model.Product, product) &&
            IsPresent(model.Category, category);
    }
    else if (category == all && status == all)
    {
        predicate = model =>
            IsPresent(model.Product, product);
    }
    
    return concessionList.Where(predicate).ToList();
}

• predicate: Я определил функциональную переменную (Func<TInput, TOutput>)
  • Он получает ConcessionModel (в качестве TInput)
  • и вернется с bool (в качестве TOutput).
• Итак, я определил функцию, реализация которой зависит от входных параметров.
• Мы используем эту функцию как условие фильтрации (Where) против источника данных (concessionList)
  • Здесь я использовал краткую форму, но ее можно было бы написать так:
    • return concessionList.Where(model => predicate(model)).ToList();

Такой подход ИМХО плохая идея. Ваш код уже почти не обслуживается и содержит множество скопированных разделов.

Вы должны передать «фильтр» (настраиваемый класс) в LoadConcessionLog() (плохое название метода, кстати) и пусть IQueryable обрабатывать различные фильтры (я предполагаю, что вы используете Entity Framework) в соответствии со строками этого ответа (или одного из других ответов на этой странице). Пусть ваш запрос применяет необходимые фильтры, чтобы данные, возвращаемые из БД, уже были ограничены.

Если вы используете Dapper, вы все равно можете применить аналогичную логику. Вы можете построить WHERE предложение в коде, или вы можете написать SQL-запрос, имеющий WHERE пункт, что принимает это во внимание.

Если вы воспользуетесь вторым подходом, у вас может быть запрос с таким WHERE;

WHERE 1 = 1
   AND (@Category IS NULL OR Category = @Category)
   AND (@Status IS NULL OR Status = @Status)

Затем вы передаете, например, new { Category = category, Status = status } куда category и status находятся strings, которые могут содержать значение или быть null.

Основные идеи:

• пусть ваша база данных выполняет фильтрацию
• напишите свой код расширяемым способом, при котором добавление нового фильтра не требует от вас копирования и вставки блоков логики.

Представьте, например, что вам нужно добавить третий фильтр. В вашем текущем коде вам нужно переосмыслить все if/else логика, вам нужно добавить этот третий фильтр к каждому сценарию, плюс вам нужно принять во внимание новые комбинации, … Это кошмар, который нужно поддерживать. Если вы примените мою логику, вам просто нужно добавить свойство в свой класс фильтра, вам нужно адаптировать WHERE предложение, вам нужно передать новый фильтр в вызове Dapper, и все.

И вы можете подумать: ну разве это не излишество, когда мне нужны только два фильтра? Возможно. Но преимущество в том, что вы приобретаете навыки, которые позже сможете применить в более крупных проектах.

Также странно, что вы используете IndexOf при сравнении Category или же Status; Я ожидал, что эти сравнения потребуют «равных». Что, если вы получите новый статус под названием «Повторно открыт»: теперь он также соответствует «Открыто» в вашей логике.

Также рассмотрите возможность нормализации вашей базы данных. «Категория» может быть отдельной таблицей, а ее идентификатор будет внешним ключом в таблице «Товары». То же самое для «Статус», хотя это также может быть enum. («Категория» может быть enum а также, конечно.)

Вот еще один более простой способ сделать фильтр:

private bool EqualsOrSource(string source, string target, bool required)
{   
    var result = target == "All"  && !required ? source : target;

    return source.Equals(result, StringComparison.InvariantCultureIgnoreCase);
}

private List<ConcessionModel> Filter(string product, string category, string status)
{
    return concessionList
        .Where(x => EqualsOrSource(x.Product, product, true) // required
    &&  EqualsOrSource(x.Category, category) // optional 
    &&  EqualsOrSource(x.Status, status)  // optional 
    ).ToList();
}