1

I am new to C# so yes this should be a faily easy question but I can't seem to find the answer to it.

I have a method that query a database.

What I am trying to do here is handle the loop though the data outside the method. 

public MySqlDataReader getDataSet(string query)
{
    MySqlDataReader dataset = null;
    MySqlConnection conn = new MySqlConnection(conn_string);

    if (startConnection(conn) == true)
    {
        MySqlCommand cmd = new MySqlCommand(query, conn);
        dataset = cmd.ExecuteReader();
        closeConnection(conn);
    }
    return dataset;
}

what I could do is write a while loop just before the closeConnection(conn); line and handle the data. But, I don't want to do it inside this method and I want to do it somewhere else in my code.

In one of my forms I want to read the database on the load so here is what I tried to do

    public newDepartment()
    {
        InitializeComponent();

        inputDepartmentName.Text = "Hi";

        dbConnetion db = new dbConnetion();
        MySqlDataReader ds = db.getDataSet("SELECT name FROM test;");

        while (ds.Read())
        {

        //Do Something

        }

    }

The problem that I am having is that I get an error Invalid attempt to Read when reader is closed

Which I belive I get this issue because I close the connection and then I am trying to read it. so What I need to do is read the data from the query and put it in an array and then loop through the array and deal with the data in a different form.

How can I workaround this issue? if my idea is good then how can I copy the data into an array and how do I loop though the array?

Here is the full class 

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using MySql.Data.MySqlClient;
using System.Windows.Forms;


namespace POS
{
    public class dbConnetion
    {
        //private OdbcConnection conn; 
        private readonly string mServer;
        private readonly string mDatabase;
        private readonly string mUid;
        private readonly string mPassword;
        private readonly string mPort;
        private readonly string conn_string;
        public dbConnetion()
        {
            mServer = "localhost";
            mDatabase = "pos";
            mUid = "root";
            mPassword = "";
            mPort = "3306";

            conn_string = String.Format("server={0};user={1};database={2};port={3};password={4};", mServer, mUid, mDatabase, mPort, mPassword);



        }

        //Start connection to database
        private bool startConnection(MySqlConnection mConnection)
        {

            try
            {
                mConnection.Open();
                return true;
            }
            catch (MySqlException ex)
            {
                MessageBox.Show(ex.Message, "Error", MessageBoxButtons.OK);
                return false;
            }

        }


        //Close connection
        private bool closeConnection(MySqlConnection mConnection)
        {
            try
            {
                mConnection.Close();
                return true;
            }
            catch (MySqlException ex)
            {
                MessageBox.Show(ex.Message);
                return false;
            }
        }

        public MySqlDataReader getDataSet(string query)
        {
            MySqlDataReader dataset = null;
            MySqlConnection conn = new MySqlConnection(conn_string);

            if (startConnection(conn) == true)
            {
                MySqlCommand cmd = new MySqlCommand(query, conn);
                dataset = cmd.ExecuteReader();
                closeConnection(conn);
            }
            return dataset;
        }


        public void processQuery(string strSQL, List<MySqlParameter> pars)
        {
            MySqlConnection conn = new MySqlConnection(conn_string);


            if (startConnection(conn) == true)
            {
                MySqlCommand cmd = new MySqlCommand(strSQL, conn);

                foreach (MySqlParameter param in pars)
                {
                    cmd.Parameters.Add(param);
                }

                cmd.ExecuteNonQuery();
                closeConnection(conn);
            }
        }
    }
}
Improve this question

2 Answers 2

Reset to default
3

Putting the records into an array would destroy the best feature of a using a datareader: that you only need to allocate memory for one record at a time. Try doing something like this:

public IEnumerable<T> getData<T>(string query, Func<IDataRecord, T> transform)
{       
    using (var conn = new MySqlConnection(conn_string))
    using (var cmd = new MySqlCommand(query, conn))
    {
        conn.Open();

        using (var rdr = cmd.ExecuteReader())
        {
           while (rdr.Read())
           {
               yield return transform(rdr);
           }
        }
    }
}

While I'm here, there's a very serious security flaw with this code and the original. A method like this that only accepts a query string, with no separate mechanism for parameters, forces you to write code that will be horribly horribly vulnerable to sql injection attacks. The processQuery() method already accounts for this, so let's extend getDataset() to avoid that security issue as well:

public IEnumerable<T> getData<T>(string query, List<MySqlParameter> pars, Func<IDataRecord, T> transform)
{      
    using (var conn = new MySqlConnection(conn_string))
    using (var cmd = new MySqlCommand(query, conn))
    {
        if (pars != null) 
        {
            foreach(MySqlParameter p in pars) cmd.Parameters.Add(p);
        }

        conn.Open();

        using (var rdr = cmd.ExecuteReader())
        {
           while (rdr.Read())
           {
               yield return transform(rdr);
           }
        }
    }
}

Much better. Now we don't have to write code that's just asking to get hacked anymore. Here's how your newDepartment() method will look now:

public newDepartment()
{
    InitializeComponent();

    inputDepartmentName.Text = "Hi";

    dbConnetion db = new dbConnetion();
    foreach(string name in db.getDataSet("SELECT name FROM test;", null, r => r["name"].ToString() ))
    {
       //Do Something
    }
}

One thing about this code is that is uses a delegate to have you provide a method to create a strongly-typed object. It does this because of the way the datareaders work: if you don't create a new object at each iteration, you're working on the same object, which can have undesirable results. In this case, I don't know what kind of object you're working with, so I just used a string based on what your SELECT query was doing.

Based on a separate discussion, here's an example of calling this for a more complicated result set:

foreach(var item in db.getDataSet(" long query here ", null, r =>
             new columnClass()
             {
                firstname = r["firstname"].ToString(),
                lastname = r["lastname"].ToString(),
                //...
             }
          ) )
{
    //Do something
}
Improve this answer
Sign up to request clarification or add additional context in comments.

13 Comments

Thanks a lot @JoelCoehoorn I am thank full for your answer. I think I may be over complicating things here which is making it difficult for me to understand the code correctly. I am sorry for being a bit slower than I should, but does your method means that I do not need a whole class to handle the connection and I should do it in functions? Therefore, my dbConnection class I can generate the db connection string and use the openConnection() top open connection and closeConnection() to close the connection but the reset should be in the form not in the dbConnection Class. correctly?
It is good to have your data access methods isolated to their own class. But you don't need methods to open and close the connection. The data access methods in the class can call Open() well enough, and you should rely on using clocks to close the connection. Just a (private) property for the connection string is good enough.
I see. can you please help me understand what does this mean IEnumerable<T> getData<T>(string query, List<MySqlParameter> pars, Func<IDataRecord, T> transform)?
It's a method named getData that returns data that you can use with a foreach loop. It makes use of generics, iterator blocks, and delegates: 3 language features well worth your time to learn.
The first parameter is the SQL query. But what is the second and the third parameters?
|
0

Since you are new to .Net I thought I point out that there are two layers of database access in ADO.Net. There are the data reader way that you are using and all of that is online only forward reading of queries. This is the lowest level access and will give you the best performance but it is more work. For most connection types you can only execute one command or have one active data reader per connection (And you can't close the connection before you have read the query as you are doing).

The other form is the offline data adapter and requires just a little bit different code, but is generally easier to use.

public DataTable getDataSet(string query)
{
    MySqlConnection conn = new MySqlConnection(conn_string);

    if (startConnection(conn) == true)
    {
        MySqlDataAdapter adapter = new MySqlDataAdapter(query, conn);
        DataTable table = new DataTable();
        adapter.Fill(table);
        closeConnection(conn);
        return table;
    }
    return null;
}

This will result in you getting a DataTable with columns and rows corresponding to the result of your query (Also look into command builders if you want to post changes back to the database later on from it, but for that you will need to keep the connection open).

One nice thing with using the data adapter is that it will figure out what the correct data types should be so you don't have to worry about invalid cast exceptions while reading the data from the data reader.

As somebody pointed out though you will need to read all the data into memory which could be a problem if you are dealing with a lot of memory. Also the DataTable class is really slow when you start dealing with a lot of records. Finally DataTable and DataSet classes also generally hook well into UI components in .Net so that their contents can easily be displayed to users.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.