1
\$\begingroup\$

So I have a grid with data and above that grid 2 comboboxes, the grid will be filled with data based on the selected values from the comboboxes. This works but can you do it way easier like I did?

enter image description here

if (_InitialDataGridPersonList != null)
{
    if (GetSelectedComboBoxPersonStateText() == "-" && GetSelectedComboBoxPersonLanguageText() != "-")
    {
        var dataGridPersonQuery = _InitialDataGridPersonList
            .Where(w => w.Language.Equals(GetSelectedComboBoxPersonLanguageText()));

        foreach (var person in dataGridPersonQuery)
        {
            _DataGridPersonList.Add(person);
        }
    }
    else if (GetSelectedComboBoxPersonStateText() != "-" && GetSelectedComboBoxPersonLanguageText() == "-")
    {
        var dataGridPersonQuery = _InitialDataGridPersonList
            .Where(w => (w.PersonState.Equals(GetSelectedComboBoxPersonStateText())));

        foreach (var person in dataGridPersonQuery)
        {
            _DataGridPersonList.Add(person);
        }
    }
    else if (GetSelectedComboBoxPersonStateText() == "-" && GetSelectedComboBoxPersonLanguageText() == "-")
    {
        foreach (var person in _InitialDataGridPersonList)
        {
            _DataGridPersonList.Add(person);
        }
    }
    else if (GetSelectedComboBoxPersonStateText() != "-" && GetSelectedComboBoxPersonLanguageText() != "-")
    {
        var dataGridPersonQuery = _InitialDataGridPersonList
            .Where(w => (w.PersonState.Equals(GetSelectedComboBoxPersonStateText()))
                    && (w.Language.Equals(GetSelectedComboBoxPersonLanguageText())));

        foreach (var person in dataGridPersonQuery)
        {
            _DataGridPersonList.Add(person);
        }
    }

Imagine there are like 10 comboboxes. So what I'm asking is, if there is a easier way with less code?

PS. I'm using MVVM so no code behind.

\$\endgroup\$
5
  • \$\begingroup\$ If your comboboxes can grow fastly, this pattern can help you. \$\endgroup\$ – Disappointed Aug 11 '17 at 11:37
  • \$\begingroup\$ Of what type is _DataGridPersonList? \$\endgroup\$ – Heslacher Aug 11 '17 at 11:55
  • \$\begingroup\$ @Heslacher DataGridPerson \$\endgroup\$ – user7998549 Aug 11 '17 at 12:00
  • \$\begingroup\$ What I mean, is e.g _DataGridPersonList a List<DataGridPerson ? \$\endgroup\$ – Heslacher Aug 11 '17 at 12:19
  • \$\begingroup\$ Yes indeed. List<DataGridPerson> \$\endgroup\$ – user7998549 Aug 11 '17 at 12:22
2
\$\begingroup\$

You have a lot of code duplication. As a first step, your code should be rewritten somehow like that:

if (_InitialDataGridPersonList != null)
{
    var personState = GetSelectedComboBoxPersonStateText();
    var personLanguage = GetSelectedComboBoxPersonLanguageText();

    var personStateSpecified = personState != "-";
    var personLanguageSpecified = personLanguage != "-";

    if (!personStateSpecified && personLanguageSpecified)
    {
        AddPersons(_InitialDataGridPersonList
                   .Where(w => w.Language.Equals(personLanguage)));
    }
    else if (personStateSpecified && !personLanguageSpecified)
    {
        AddPersons(_InitialDataGridPersonList
                   .Where(w => w.PersonState.Equals(personState)));
    }
    else if (!personStateSpecified && !personLanguageSpecified)
    {
        AddPersons(_InitialDataGridPersonList);
    }
    else if (personStateSpecified && personLanguageSpecified)
    {
        AddPersons(_InitialDataGridPersonList
                   .Where(w => w.PersonState.Equals(personState) &&
                               w.Language.Equals(personLanguage)));
    }
}

private void AddPersons(IEnumerable<Person> persons)
{
    foreach (var person in persons)
    {
        _DataGridPersonList.Add(person);
    }
}

As a second step, you can place all comboboxes in a map where you'll link each combobox with property selector:

public enum PropertyType
{
    State,
    Language
}

private static readonly Dictionary<PropertyType, Func<Person, object>> _propertySelectors =
    new Dictionary<PropertyType, Func<Person, object>>
    {
        [PropertyType.State] = p => p.PersonState,
        [PropertyType.Language] = p => p.Language
    }

private void PopulatePersons()
{
    if (_InitialDataGridPersonList != null)
    {
        var propertyValues = Enum.GetValues(typeof(PropertyType))
                                 .Cast<PropertyType>()
                                 .ToDictionary(t => t,
                                               t => GetComboBoxText(t));

        AddPersons(_InitialDataGridPersonList.Where(p =>
        {
            return propertyValues.All(v => v.Value == "-" ||
                                           _propertySelectors[v.Key](p).Equals(v.Value)));
        });
    }
}

private void AddPersons(IEnumerable<Person> persons)
{
    foreach (var person in persons)
    {
        _DataGridPersonList.Add(person);
    }
}

private string GetComboBoxText(PropertyType propertyType)
{
    switch (propertyType)
    {
        case PropertyType.State:
            return GetSelectedComboBoxPersonStateText();
        case PropertyType.Language:
            return GetSelectedComboBoxPersonLanguageText();
        ...
    }

    return "-";
}

This approach allows you to add another comboboxes without a pain.

\$\endgroup\$
11
  • \$\begingroup\$ Hey, looks better but not working. Is there also a way to make the if else statements better or shorter? \$\endgroup\$ – user7998549 Aug 11 '17 at 12:01
  • \$\begingroup\$ @user7998549 but not working and it doesn't have to ;-) It can be a suggestion. It's not required to present working code especially what so many unknowns as in our example \$\endgroup\$ – t3chb0t Aug 11 '17 at 12:10
  • \$\begingroup\$ If you about first step there is nothing more there than you have provided in your original code. I've added second step which allows you to scale your app to more comboboxes. \$\endgroup\$ – Maxim Aug 11 '17 at 12:13
  • \$\begingroup\$ @Maxim hmm nice thats works but only if I comment this part (the filter part :p) '_propertySelectors[v.Key](p).Equals(v.Value))));' \$\endgroup\$ – user7998549 Aug 11 '17 at 12:48
  • \$\begingroup\$ @user7998549 It is strange. Can you say, was your original code working? :) Because the code you've commented is necessary to select persons with properties values selected in comboboxes. \$\endgroup\$ – Maxim Aug 11 '17 at 13:17
0
\$\begingroup\$

You might want to abstract your filters out. It does not require that much effort/extra code, yet it improves readability drastically.

interface IPersonFilter
{
    bool TryPass(Person person);
}

class StateFilter : IPersonFilter
{
    //You can implement INotifyPropertyChanged and bind combobox's 
    //Text or SelectedItem directly to this property, if you want to. 
    //That would simplify parent viewmodel. 
    public string State { get; set; }

    public bool TryPass(Person person)
    {
        if (State == "-") return true;
        return person.PersonState == State;
    }
}

//etc.

You can then construct your list as:

IPersonFilter[] allFilters = new IPersonFilter[] //this should be a property
{ 
    new StateFilter(), 
    new LanguageFilter() 
}; 
var filtered = _InitialDataGridPersonList.Where(p => allFilters.All(f => f.TryPass(p)));
_dataGridPersonList = new ObservableCollection<Person>(filtered);

P.S. In case you are not aware, wpf supports basic filtering out of the box. In order to use it you should create a CollectionViewSource and set its Filter delegate:

CollectionSource = CollectionViewSource.GetDefaultView(_InitialDataGridPersonList);
CollectionSource.Filter = p => 
{
    var person = (Person)p;

    //return true to display person, false - otherwise.

    //using above implementation:
    return allFilters.All(f => f.TryPass(p));
};

Then you can remove _dataGridPersonList, bind your grid to CollectionSource property instead and call CollectionSource.DeferRefresh() when filters change to update the view.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.