5
\$\begingroup\$

This code get a list<dynamic> 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<dynamic> 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<dynamic>(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 ?

\$\endgroup\$
  • \$\begingroup\$ Why use dynamic instead of a custom class if that is giving you issues? \$\endgroup\$ – BCdotWEB May 10 '16 at 14:52
  • \$\begingroup\$ It's part of the question, I would like to know if there's a way to bind the list directly to the combobox without using a custom class. \$\endgroup\$ – SilentRage47 May 10 '16 at 14:59
5
\$\begingroup\$

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<Material>, not a List<dynamic>. First an IReadOnlyList<T> 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<T> would work fine, but an IReadOnlyList<T> 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<dynamic>(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.

\$\endgroup\$
  • \$\begingroup\$ I have indeed a Material class but it takes more time to get a list of Material object. My primary objective here is populate the combo in the fastest way possibile. \$\endgroup\$ – SilentRage47 May 11 '16 at 7:39
  • \$\begingroup\$ I didn't know you could create a class object without all the fields! I indeed have a Material class and you're method works like a charm. I tried returning a List of Material objects before but it was taking a long time since I was creating the full object with fields I didn't need. Thanks for the tips. \$\endgroup\$ – SilentRage47 May 11 '16 at 8:00
  • \$\begingroup\$ @SilentRage47 it works because the type is mutable and there's a default constructor ;-) \$\endgroup\$ – Mathieu Guindon May 11 '16 at 17:18
3
\$\begingroup\$

I didn't use Drapper, but I believe that your problem is caused by trying to put dynamic type objects into a combo box. AFAIK ComboBox uses a reflection mechanism to get ValueMember and DisplayMember. I see that running a reflection for dynamic type can be a little problematic.

Your idea with switching a type to put it into the ComboBox is the proper way to go, but You can do it even easier with using an anonymous type. This way the file ComboBoxItem.cs will not be necessary, and the reflection inside ComboBox guts will work properly.

var mRep = new MaterialRepository();

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

I strongly suggest switching from WinForms to WPF and MVVM pattern.

\$\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.