Answered ExecuteReader not working

PDS8475

Active member
Joined
Jun 25, 2019
Messages
33
Programming Experience
Beginner
I have a database table called Sites with to columns SiteName and SiteAddress
The following code is on a button click event.
I have removed the database password from the connection string intentionally before posting.
When I step through the code, I can see the connection state opens.
However the reader remains null and when the ExecuteReader is called it jumps to the catch block.
the catch block gives the exception message "Failed due to No value given for one or more required parameters"

It appears to me that the query is wrong and therefore the reader can't read, But I can not see how it is wrong. I tried copying and pasting the names of the Table and columns into the query to make sure that I have them exactly right, I've also checked with other update queries online and it seems to be right.
Could anybody please tell me what is going wrong as I have spent two days on this, trying different things and I am getting no where.
C#:
            string sn = SiteName_textBox.Text;
            string sa = SiteAddress_textBox.Text;
            try
            {
               OleDbDataReader read;
               OleDbConnection connection = new OleDbConnection(@"Provider = Microsoft.Jet.OLEDB.4.0; Data Source = |DataDirectory|\Admins.mdb; Jet OLEDB:Database Password = ");
               
               string my_query = "UPDATE Sites SET SiteAddress = @sa WHERE SiteName = @sn";
               OleDbCommand comm = new OleDbCommand(my_query, connection);
               connection.Open();
               read = comm.ExecuteReader();
               if (read.HasRows)
               {
                   while (read.Read())
                   {
                      comm.Parameters.AddWithValue("@sn", SiteName_textBox.Text);
                      comm.ExecuteNonQuery();
                   }
               }
               read.Close();
               connection.Close();
            }
            catch (Exception ex)
            {
                MessageBox.Show("Failed due to " + ex.Message);
            }
 
Last edited:

jmcilhinney

C# Forum Moderator
Staff member
Joined
Apr 23, 2011
Messages
2,431
Location
Sydney, Australia
Programming Experience
10+
Your code makes no sense. As you'd expect, the ExecuteReader method is used to read data, which requires a SELECT statement. If that's not what you're doing then don't call that method. If you want execute an UPDATE statement then just call ExecuteNonQuery. There's nothing to read so don;t try to read anything.
 

PDS8475

Active member
Joined
Jun 25, 2019
Messages
33
Programming Experience
Beginner
Your code makes no sense. As you'd expect, the ExecuteReader method is used to read data, which requires a SELECT statement. If that's not what you're doing then don't call that method. If you want execute an UPDATE statement then just call ExecuteNonQuery. There's nothing to read so don;t try to read anything.
If I have
C#:
            string sn = SiteName_textBox.Text;
            string sa = SiteAddress_textBox.Text;
            try
            {
               OleDbConnection connection = new OleDbConnection(@"Provider = Microsoft.Jet.OLEDB.4.0; Data Source = |DataDirectory|\Admins.mdb; Jet OLEDB:Database Password = ");
               string my_query = "UPDATE Sites SET SiteAddress = @sa WHERE SiteName = @sn";
               OleDbCommand comm = new OleDbCommand(my_query, connection);
               connection.Open();
               comm.Parameters.AddWithValue("@sn", SiteName_textBox.Text);
               comm.Parameters.AddWithValue("@sa", SiteAddress_textBox.Text);
               comm.ExecuteNonQuery();
               connection.Close();
            }
            catch (Exception ex)
            {
                MessageBox.Show("Failed due to " + ex.Message);
            }
in other words take everything pertaining to the reader out. I don't get an exception error but the table record still doesn't get updated.
 

Skydiver

Well-known member
Joined
Apr 6, 2019
Messages
461
Location
Virginia Beach, VA
Programming Experience
10+
From the OleDbParameter documentation says:
The OLE DB.NET Framework Data Provider uses positional parameters that are marked with a question mark (?) instead of named parameters.
Ironically, the sample code in the documentation uses @name rather than the positional ?.
 

Skydiver

Well-known member
Joined
Apr 6, 2019
Messages
461
Location
Virginia Beach, VA
Programming Experience
10+
From a 2008 blogpost, apparently the order matters. Try swapping your lines 9 and 10 so that you add the parameters in the same order as found in the query.
 

PDS8475

Active member
Joined
Jun 25, 2019
Messages
33
Programming Experience
Beginner
Thank you to both of you. I didn't realise the order of the parameters mattered.
 

Skydiver

Well-known member
Joined
Apr 6, 2019
Messages
461
Location
Virginia Beach, VA
Programming Experience
10+
The order shouldn't matter for most sane databases. Unfortunately, Microsoft has to keep Access alive and they do the best that they can given how old Access is, as well as the vagaries of the OLEDB adapter for Access. OLEDB lost the ODBC vs OLEDB war.

If you don't really need an Access database, consider SQLite or SQL Express.
 

jmcilhinney

C# Forum Moderator
Staff member
Joined
Apr 23, 2011
Messages
2,431
Location
Sydney, Australia
Programming Experience
10+
By the way, you should be closing your connection in a finally block rather than in the try block. As it is, your connection will remain open if an exception is thrown. Better still, create it with a using statement and then it is guaranteed to be closed at the end of the block, even if an exception is thrown and not caught.
 

Sheepings

Senior Programmer
Joined
Sep 5, 2018
Messages
466
Location
UK
Programming Experience
10+
Just for fun, I wanted to contribute a point of interest and have a healthy discussion on : try/catch + using, right syntax and hopefully the comments will also help our op determine best practice. I know we all have our own preferences, and so I wonder what your thoughts on what the syntax usage should be. I'll bring the question into the topic for convenience, and add finally statement for completeness since it wasn't used in the original question. :
Which one
C#:
            try
            {
                using (var myObject = new MyClass())
                {
                    // something here...
                }
            }
            catch (Exception ex)
            {
                // Handle exception

            }
            finally
            {
                // Finish up
            }
OR
C#:
            using (var myObject = new MyClass())
            {
                try
                {
                    // something here...
                }
                catch (Exception ex)
                {
                    // Handle exception
                }
                finally
                {
                    //Finish up
                }
            }
Take note there is no mention of finally in the original topic, so I've added them here. Should using come before try, or after it? I mostly find myself following the first principle block as it has served me well, but I can see the reasoning behind both the arguments made on that topic. What are your thoughts? Chime in don't be shy
 

Skydiver

Well-known member
Joined
Apr 6, 2019
Messages
461
Location
Virginia Beach, VA
Programming Experience
10+
I tend to write code using the former mostly as a carry over from my C++ days.

Before C# 7.0, I would write things like this:
Code:
public IEnumerable<Record> DoSomething(int someParam, string otherParam)
{
    try
    {
        return DoSomethingReally(someParam, otherParam);
    }
    catch (SomeException ex)
    {
        Log.Error(ex);
        // throw or return empty depending on if error is recoverable.
    }
}

IEnumerable<Record> DoSomethingReally(int someParam, string otherParam)
{
    using (var someObj = new Some(someParam))
    using (var otherObj = new Other(otherParam))
    {
        // do things here
    }
}
With C# 7.0, I've been using this style more and more:
Code:
public IEnumerable<Record> DoSomething(int someParam, string otherParam)
{
    try
    {
        return DoSomethingReally();
    }
    catch (SomeException ex)
    {
        Log.Error(ex);
        // throw or return empty depending on if error is recoverable.
    }

    IEnumerable<Record> DoSomethingReally()
    {
        using (var someObj = new Some(someParam))
        using (var otherObj = new Other(otherParam))
        {
            // do things here
        }
    }
}
It's very likely with C# 8.0, I'll even transition to this style:
Code:
public IEnumerable<Record> DoSomething(int someParam, string otherParam)
{
    try
    {
        return DoSomethingReally();
    }
    catch (SomeException ex)
    {
        Log.Error(ex);
        // throw or return empty depending on if error is recoverable.
    }

    IEnumerable<Record> DoSomethingReally()
    {
        using var someObj = new Some(someParam);
        using var otherObj = new Other(otherParam);
        // do things here
    }
}
OR if the DoSomethingReally() is really short, then:
Code:
public IEnumerable<Record> DoSomething(int someParam, string otherParam)
{
    try
    {
        using var someObj = new Some(someParam);
        using var otherObj = new Other(otherParam);
        // do things here
    }
    catch (SomeException ex)
    {
        Log.Error(ex);
        // throw or return empty depending on if error is recoverable.
    }
}
Anyway, the point is that I try to keep all the "do something" code focused on doing the operation along the happy path. Any errors or exception handling is outside of that happy path and is handled elsewhere. I broke out the DoSomethingReally() to try to reduce the indentation depth, although admittedly the local methods of C# 7.0 doesn't really help much in that regard.
 
Last edited:
Top Bottom