Функциональность фильтрации 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; }
}

3 ответа
3

Позвольте мне сосредоточиться на 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();

  • В Func<ConcessionModel, bool> predicate = model это не то, что я видел раньше, вы можете объяснить, пожалуйста, что происходит? 🙂

    – LV98


  • @ LV98 Я расширил свой ответ, пожалуйста, проверьте его еще раз.

    – Питер Чала

  • Быстрый вопрос … скажем, у меня есть логическое значение в моей модели, которое называется Archived. Новый параметр будет добавлен в Фильтр как bool archived, но IsPresent покажет ошибку, из которой невозможно преобразовать bool to string. Как бы вы изменили текущий код?

    – LV98


  • 1

    @ LV98 IsPresent был разработан для сравнения строк. Если вы хотите выполнить фильтрацию с помощью дополнительной переменной типа bool, вы можете сделать что-то вроде этого: predicate = model => IsPresent(model.Product, product) && model.Archived == archived. Это отвечает на ваш вопрос?

    – Питер Чала

  • Ах, понятно … Спасибо

    – LV98

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

Вы должны передать “фильтр” (настраиваемый класс) в 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 а также, конечно.)

  • Я использую dapper вместо Entity, извините, о чем следует упомянуть.

    – LV98

  • @ LV98 Я также добавил раздел WRT Dapper.

    – BCdotWEB

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

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();
}

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *