List<dynamic> to ComboBox

This code get a list from a repository class using Dapper. I need to bind this list to a ComboBox. The only way I can get this to work is by using a simple class (ComboBoxItem in my case) and than use that to do the binding.

UI.cs

var mRep= new MaterialRepository();

comboBox1.Items.Clear();
foreach (var item in listMaterial)
{
  comboBox1.Items.Add(new ComboBoxItem(Convert.ToString(item.code),Convert.ToString(item.value)));
}
comboBox1.ValueMember = "value";
comboBox1.DisplayMember = "name";

MaterialRepository.cs

public List GetList(int idQuery)
{
    using (DatabaseConnection db = new DatabaseConnection()) //Open a MySQLConnection
    {
         const string query = @"SELECT code,value FROM table1 WHERE id = @Id";
         var par = new {Id = idQuery};

         return db.con.Query(query, par).ToList();
    }
}

ComboBoxItem.cs

public class ComboBoxItem
{
    public string name;
    public string value;

    public ComboBoxItem(string name,string value)
    {
        this.name = name;
        this.value = value;
    }

    public override string ToString()
    {
        return name;
    }
}

Is there any other way to achieve this ?

dynamic has this tendency of making everything that touches it dynamic as well, which pretty much nullifies type safety and all its benefits: you want to cage that monster as early as possible, and avoid leaking it everywhere.

A MaterialRepository should expose an IReadOnlyList, not a List. First an IReadOnlyList because you don't want the client code to have the right to treat the query results as a list (think Add/Remove), but merely as something they can iterate over: an IEnumerable would work fine, but an IReadOnlyList is better because then the client code won't trigger any warnings when it tries to iterate it more than once - which is something you have no control over from the repository class. And since you're already doing .ToList(), might as well expose it as such to your clients.

What you're missing is a proper abstraction for your materials; what if you eventually need more than just Code and Value? With a Material class, you could do this:

return db.con
         .Query(query, par)
         .Select(item => new Material
             {
                 Code = item.Code.ToString(), 
                 Value = item.Value.ToString()
             })
         .ToList();

And then you no longer need that ComboBoxItem class - instead you can have Material instances in your combo box! ..and if you have more than just a Code and a Value to display, you can get all that information at once, and reuse it.

Also you should consider using a BindingList to data-bind your items instead of manually adding them:

var materialsBindingList = new BindingList(materials);
comboBox1.DataSource = materialsBindingList;
comboBox1.ValueMember = "Value";
comboBox1.DisplayMember = "Code";

That would pave the way to more modern UI frameworks, like Windows Presentation Foundation (WPF), where data binding is your go-to way of feeding data into a UI.

Name things. comboBox1 needs a name. It contains a material that the user selected. SelectedMaterial seems like a much more meaningful name for it.

In this snippet:

var mRep= new MaterialRepository();

comboBox1.Items.Clear();
foreach (var item in listMaterial)

It's not clear where listMaterial is coming from, and the coupling between what appears to be your UI (/form) and that MaterialRepository class isn't ideal either. Why instantiate a new MaterialRepository every time you need to call into it?

WinForms works well with a Model-View-Presenter pattern; you make a class to hold your "model" (exposing the available materials, user selections, etc.), and then the "view" uses it to get its data and store the state of things; the "presenter" coordinates everything, populates the "model", feeds it to the "view" and knows how and when to display and tear down the UI.

Refactoring toward MVP would help decouple your components and help with separating the application logic from the data access and UI components.

StackExchange Code Review Q#127991, answer score: 5