Question why am i getting insufficient parameters supplied to the command error

rowlandsfc

Active member
Joined
Feb 3, 2019
Messages
43
Location
bridgend
Programming Experience
Beginner
i have trying to read a database and display it into a datagridview but i keep getting the error
C#:
private void nameSearch()
{
    using (SQLiteCommand cmd = connAccount.CreateCommand())
    {
        connAccount.Open();
        try
        {
            connAccount = new SQLiteConnection(dbConnection.source);

            cmd.CommandText = @"SELECT accid, account.custid, customer.title, customer.firstname || ' ' || customer.lastname AS Name, product.isaname AS ProductName, balance, accrued, active FROM account INNER JOIN customer ON customer.custid = account.custid INNER JOIN product ON product.prodid = account.prodid WHERE Name = @setName";

            cmd.Parameters.AddWithValue("@setName", txt_name.Text);
            cmd.ExecuteNonQuery();

            da_Accounts = new SQLiteDataAdapter(cmd.CommandText, connAccount);
            dt_Accounts = new DataTable();
            da_Accounts.Fill(dt_Accounts);
            dgv_Account.DataSource = dt_Accounts;
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
    }
}
 
Last edited by a moderator:

jmcilhinney

C# Forum Moderator
Staff member
Joined
Apr 23, 2011
Messages
3,720
Location
Sydney, Australia
Programming Experience
10+
You really ought to spend a bit more time learning the basics of ADO.NET because there's a lot wrong there. Code like that is what happens when people don't think about what the code actually has to do before writing it. I'm not talking about just the end result but the steps to get there. If you think about the logic first and work out the steps that need to be performed, then write code to implement those steps specifically, you won't end up with code doing pointless things or not doing required things. Let's take a look at that code and think about what it actually does.
C#:
using (SQLiteCommand cmd = connAccount.CreateCommand())
That line creates a new command object. OK.
C#:
cmd.CommandText = @"SELECT accid, account.custid, customer.title, customer.firstname || ' ' || customer.lastname AS Name, product.isaname AS ProductName, balance, accrued, active FROM account INNER JOIN customer ON customer.custid = account.custid INNER JOIN product ON product.prodid = account.prodid WHERE Name = @setName";
That line sets the SQL code for that command to a SELECT statement, i.e. a query. OK
C#:
cmd.Parameters.AddWithValue("@setName", txt_name.Text);
That line adds a parameter to the command. Why you would be using "@setName" instead of just "@Name" when the parameter is specifically to match the Name column is beyond me, but bad names will still work so we're still OK.
C#:
cmd.ExecuteNonQuery();
Now you have gone off the rails. This is what I mean about code doing pointless things. What exactly is that line supposed to be achieving? If you wrote an algorithm, what step would that be performing? The ExecuteNonQuery method is specifically for executing SQL statements that do not produce a result set, i.e. non-queries. What does a SELECT statement do? It produces a result set, i.e. it's a query. You obviously want the contents of that result set so what is the reason that you are calling ExecuteNonQuery? There isn;t one. You have code for no reason.
C#:
da_Accounts = new SQLiteDataAdapter(cmd.CommandText, connAccount);
Here you go further off the rails. You create a data adapter using the same SQL query as you used to create the prior command but you don't actually use that command. You're discarding the command you already had - you know, the one you added the parameter to - and telling the data adapter to create a new command. You never add any parameters to that command so why be surprised when you're told that you have insufficient parameters?

What you need to do is create a single command, add a parameter to it, execute that one command once and then get the result set. That means getting rid of the ExecuteNonQuery call for a start. It also mean either using the command you already have when creating the data adapter or letting the data adapter create the command first and then adding the parameter to that. There's also no point opening the connection because Fill will open it implicitly if it's not already open. It will also close it again if it had to open it, which is just as well, given that you code opens the connection but doesn't close it.

I also just realised that the code you have uses an existing connection to create a command and then opens that connection, then it creates a new connection object! What madness is that? If you're not using a dedicated data access layer (DAL) that creates all the data access objects internally, you really ought to create all the objects where and when you need them. Create your connection object, create your command and/or data adapter objects, open the connection if required, execute the SQL, close the connection if required. Simple and repeatable.
C#:
private void SearchByName()
{
    var query = @"SELECT accid,
                         account.custid,
                         customer.title,
                         customer.firstname || ' ' || customer.lastname AS Name,
                         product.isaname AS ProductName,
                         balance,
                         accrued,
                         active
                  FROM account INNER JOIN customer
                  ON customer.custid = account.custid INNER JOIN product
                  ON product.prodid = account.prodid
                  WHERE Name = @Name";
    
    using (var connection = new SQLiteConnection(dbConnection.source))
    using (var adapter = new SQLiteDataAdapter(query, connection))
    {
        try
        {
            adapter.SelectCommand.Parameters.AddWithValue("@Name", txt_name.Text);
            dt_Accounts = new DataTable();
            da_Accounts.Fill(dt_Accounts);
            dgv_Account.DataSource = dt_Accounts;
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
    }
}
There are variations on that that would also work but the code is simple and readable and uses sensible, self-documenting names.
 

rowlandsfc

Active member
Joined
Feb 3, 2019
Messages
43
Location
bridgend
Programming Experience
Beginner
You really ought to spend a bit more time learning the basics of ADO.NET because there's a lot wrong there. Code like that is what happens when people don't think about what the code actually has to do before writing it. I'm not talking about just the end result but the steps to get there. If you think about the logic first and work out the steps that need to be performed, then write code to implement those steps specifically, you won't end up with code doing pointless things or not doing required things. Let's take a look at that code and think about what it actually does.
C#:
using (SQLiteCommand cmd = connAccount.CreateCommand())
That line creates a new command object. OK.
C#:
cmd.CommandText = @"SELECT accid, account.custid, customer.title, customer.firstname || ' ' || customer.lastname AS Name, product.isaname AS ProductName, balance, accrued, active FROM account INNER JOIN customer ON customer.custid = account.custid INNER JOIN product ON product.prodid = account.prodid WHERE Name = @setName";
That line sets the SQL code for that command to a SELECT statement, i.e. a query. OK
C#:
cmd.Parameters.AddWithValue("@setName", txt_name.Text);
That line adds a parameter to the command. Why you would be using "@setName" instead of just "@Name" when the parameter is specifically to match the Name column is beyond me, but bad names will still work so we're still OK.
C#:
cmd.ExecuteNonQuery();
Now you have gone off the rails. This is what I mean about code doing pointless things. What exactly is that line supposed to be achieving? If you wrote an algorithm, what step would that be performing? The ExecuteNonQuery method is specifically for executing SQL statements that do not produce a result set, i.e. non-queries. What does a SELECT statement do? It produces a result set, i.e. it's a query. You obviously want the contents of that result set so what is the reason that you are calling ExecuteNonQuery? There isn;t one. You have code for no reason.
C#:
da_Accounts = new SQLiteDataAdapter(cmd.CommandText, connAccount);
Here you go further off the rails. You create a data adapter using the same SQL query as you used to create the prior command but you don't actually use that command. You're discarding the command you already had - you know, the one you added the parameter to - and telling the data adapter to create a new command. You never add any parameters to that command so why be surprised when you're told that you have insufficient parameters?

What you need to do is create a single command, add a parameter to it, execute that one command once and then get the result set. That means getting rid of the ExecuteNonQuery call for a start. It also mean either using the command you already have when creating the data adapter or letting the data adapter create the command first and then adding the parameter to that. There's also no point opening the connection because Fill will open it implicitly if it's not already open. It will also close it again if it had to open it, which is just as well, given that you code opens the connection but doesn't close it.

I also just realised that the code you have uses an existing connection to create a command and then opens that connection, then it creates a new connection object! What madness is that? If you're not using a dedicated data access layer (DAL) that creates all the data access objects internally, you really ought to create all the objects where and when you need them. Create your connection object, create your command and/or data adapter objects, open the connection if required, execute the SQL, close the connection if required. Simple and repeatable.
C#:
private void SearchByName()
{
    var query = @"SELECT accid,
                         account.custid,
                         customer.title,
                         customer.firstname || ' ' || customer.lastname AS Name,
                         product.isaname AS ProductName,
                         balance,
                         accrued,
                         active
                  FROM account INNER JOIN customer
                  ON customer.custid = account.custid INNER JOIN product
                  ON product.prodid = account.prodid
                  WHERE Name = @Name";
   
    using (var connection = new SQLiteConnection(dbConnection.source))
    using (var adapter = new SQLiteDataAdapter(query, connection))
    {
        try
        {
            adapter.SelectCommand.Parameters.AddWithValue("@Name", txt_name.Text);
            dt_Accounts = new DataTable();
            da_Accounts.Fill(dt_Accounts);
            dgv_Account.DataSource = dt_Accounts;
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
    }
}
There are variations on that that would also work but the code is simple and readable and uses sensible, self-documenting names.
Hey I really appreciate you going into hat much detail, ill go through it when I get a chance and take a look at the link provided, everything I've done so far is based on what the college is teaching and I'm slowly learning that what they teach isn't great lol
 
Top Bottom