2
\$\begingroup\$

I would like to improve the function below without calling twice InserisciRigaNote.

Before I fill my dictionary. When I compare idNota with the current database id I check if it is equal or not, if different means that the dictionary is filled and I can proceed with the insertion of the row in DataGridView

I have a query that return this output:

Output MySQL Query

I have to fill a DataGridView in that way

DataGridView with language

string query = ""; // Query MySQL              
using (MySqlDataReader reader = db.ExecuteReader(query))
{
    if (reader.HasRows)
    {
        int idNota = 1;
        var note = new Dictionary<string, string>();
        while (reader.Read())
        {
            if (idNota != Convert.ToInt32(reader["id_nota_lavaggio"]))
            {
                InserisciRigaNote(note, idNota); // Function that insert row in the DataGridView

                idNota = Convert.ToInt32(reader["id_nota_lavaggio"]);
                note.Clear();
            }
            // Create a dictionary made by a language key and a description
            note[reader["id_lingua"].ToString()] = reader["descrizione_nota_lavaggio"].ToString();
        }
        if (note.Count > 0) // If there is something in the dictionary
        {
            InserisciRigaNote(note, idNota);
            note.Clear();
        }

    }
}

N.B. I did not use Pivot to make the query because I need it's all completely dynamic

\$\endgroup\$
1
  • \$\begingroup\$ You can do dynamic pivot queries too. \$\endgroup\$
    – RubberDuck
    Mar 3 '16 at 17:34
3
\$\begingroup\$

Your code is really confusing, not in the least because it is partly in Italian.


It does seem to me that you've coupled your UI and your back-end, and that's something to avoid. Look into developing an n-tier application so your db related code is in a back-end and all you're exposing to the UI is a collection of some sort with data objects.

And the you can bind that collection to your DataGridView, which is a much nicer solution instead of inserting one row at a time. Ideally you'd abandon WinForms, instead look into more modern technologies like WPF and apply MVVM. But if you're stuck with WinForms there are still ways to apply something like MVP.


That is of course a massive leap from the code you have here, so try refactoring one bit at the time.

  • Construct a class that can contain the results of your db-query, and fill a collection, so you can properly bind your data instead of inserting rows.
  • Beware of strings like "id_nota_lavaggio" and consider defining them as const so you can easily update them if necessary.
  • Twice you do Convert.ToInt32(reader["id_nota_lavaggio"]): instead, do it once and store the result in a variable.
  • I don't know about how well Entity Framework works with MySQL, but there are other ORMs available anyway, so when possible consider abandoning writing ADO and instead work with an ORM.
  • Comments should tell us why, not what. // Query MySQL is superfluous, and something like // Create a dictionary made by a language key and a description should be told by the code, not the comments (ditto // If there is something in the dictionary).
\$\endgroup\$
2
\$\begingroup\$

Use a pivot query to select the data from the database in the correct format. Then you don't need to use a dictionary, but just a regular old List which you can bind to.

dataGridView.DataSource = myPivotedQueryResults
\$\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.