2

I have a stored procedure for inserting a record in a table. I'm trying to use Scope_Identity() to get back the created ID value. I call the procedure from a method named Add_Claimant(). The method inserts a claimant into the table fine, but I am not getting the ID back.

To make this work, I added @ID int output to the parameter list for the stored procedures. After the VALUES part of the Insert statement I also added SET @ID=SCOPE_IDENTITY(). Then in my method, I added the @ID parameter, like this:

cmd.Parameters.Add("@ID", SqlDbType.Int).Direction = ParameterDirection.Output;

I open the connection and execute the stored procedure as normal, and then retrieve the parameter value like this:

connection.Open();          
cmd.ExecuteNonQuery();      
String id = cmd.Parameters["@ID"].Value.ToString(); 
this.ClmntTbl_ID = Convert.ToInt32(id);

At this point I expect to see the actual value from the output parameter in the ClmntTbl_ID member. However, I get the following exception:

System.FormatException: 'Input string was not in a correct format.'

If I mouse over id it shows its value as "" (an empty string).

What am I missing?

Here is the full method.

//Connection that is passed from the calling program.
SqlConnection My_Connection = new SqlConnection(ConnectionString);

public void Add_Claimant(SqlConnection connection)
{
    string id;

    // Build the parameter string to pass to the stored procedure.
    String Param_String;
    Param_String = "@SSN, @LEGACYID, @FIRSTNAME, @LASTNAME, @MIDDLEI, @HOMEPHONE, @CELLPHONE, @BIRTHDATE";
    Param_String = Param_String + ", @SEX, @RACECODE, @ETHNICCODE, @MARITALSTATUS, @EDULEVEL, @CITIZENCODE";
    Param_String = Param_String + ", @LEGACYPIN, @PASSWORDVALUE, @HANDCAP_IND, @LATEST_RTWDATE, @LATEST_RECALLTWDATE";
    Param_String = Param_String + ", @LATEST_NEWHIRE_EMP_ACCT, @LATEST_NEWHIREDATE, @DECEASED_IND, @ALIENREG_NUM, @ALIENREG_EXPDATE, @PAYMETHOD, @ID";

    SqlCommand cmd = connection.CreateCommand();
    cmd.CommandText = "Execute ADD_CLAIMANT_POC " + Param_String;   //Call the stored procedure ADD_CLAIMANT_POC

    cmd.Parameters.Add("@SSN", SqlDbType.VarChar, 9).Value = this.ClmntTbl_SSN;
    cmd.Parameters.Add("@LEGACYID", SqlDbType.VarChar, 9).Value = this.ClmntTbl_LEGACYCID;
    cmd.Parameters.Add("@FIRSTNAME", SqlDbType.VarChar, 50).Value = this.ClmntTbl_FIRSTNAME;
    cmd.Parameters.Add("@LASTNAME", SqlDbType.VarChar, 50).Value = this.ClmntTbl_LASTNAME;
    cmd.Parameters.Add("@MIDDLEI", SqlDbType.VarChar, 1).Value = this.ClmntTbl_MIDDLEI;
    cmd.Parameters.Add("@HOMEPHONE", SqlDbType.VarChar, 10).Value = this.ClmntTbl_HOMEPHONE;
    cmd.Parameters.Add("@CELLPHONE", SqlDbType.VarChar, 10).Value = this.ClmntTbl_CELLPHONE;
    cmd.Parameters.Add("@BIRTHDATE", SqlDbType.DateTime).Value = this.ClmntTbl_BIRTHDATE;
    cmd.Parameters.Add("@SEX", SqlDbType.Char, 1).Value = this.ClmntTbl_SEX;
    cmd.Parameters.Add("@RACECODE", SqlDbType.Char, 1).Value = this.ClmntTbl_RACECODE;
    cmd.Parameters.Add("@ETHNICCODE", SqlDbType.Char, 1).Value = this.ClmntTbl_ETHNICCODE;
    cmd.Parameters.Add("@MARITALSTATUS", SqlDbType.Char, 1).Value = this.ClmntTbl_MARITALSTATUS;
    cmd.Parameters.Add("@EDULEVEL", SqlDbType.Int).Value = this.ClmntTbl_EDULEVEL;
    cmd.Parameters.Add("@CITIZENCODE", SqlDbType.Char, 1).Value = this.ClmntTbl_CITIZENCODE;
    cmd.Parameters.Add("@LEGACYPIN", SqlDbType.Int).Value = this.ClmntTbl_LEGACYPIN;
    cmd.Parameters.Add("@PASSWORDVALUE", SqlDbType.VarChar, 50).Value = this.ClmntTbl_PASSWORDVALUE;
    cmd.Parameters.Add("@HANDCAP_IND", SqlDbType.Char, 1).Value = this.ClmntTbl_HANDICAP_IND;
    cmd.Parameters.Add("@LATEST_RTWDATE", SqlDbType.DateTime).Value = this.ClmntTbl_LATEST_RTWDATE;
    cmd.Parameters.Add("@LATEST_RECALLTWDATE", SqlDbType.DateTime).Value = this.ClmntTbl_LATEST_RECALLTWDATE;
    cmd.Parameters.Add("@LATEST_NEWHIRE_EMP_ACCT", SqlDbType.VarChar, 18).Value = this.ClmntTbl_LATEST_NEWHIRE_EMP_ACCT;
    cmd.Parameters.Add("@LATEST_NEWHIREDATE", SqlDbType.DateTime).Value = this.ClmntTbl_LATEST_NEWHIREDATE;
    cmd.Parameters.Add("@DECEASED_IND", SqlDbType.Char, 1).Value = this.ClmntTbl_DECEASED_IND;
    cmd.Parameters.Add("@ALIENREG_NUM", SqlDbType.VarChar, 9).Value = this.ClmntTbl_ALIENREG_NUM;
    cmd.Parameters.Add("@ALIENREG_EXPDATE", SqlDbType.DateTime).Value = this.ClmntTbl_ALIENREG_EXPDATE;
    cmd.Parameters.Add("@PAYMETHOD", SqlDbType.Char, 1).Value = this.ClmntTbl_PAYMETHOD;
    cmd.Parameters.Add("@ID", SqlDbType.Int).Direction = ParameterDirection.Output;

    if (connection != null && connection.State == ConnectionState.Closed)
    {
        connection.Open();    //if it was not opened in the calling program, or if something strange happened and it was closed open the connection.

        cmd.ExecuteNonQuery();      //Execute stored procedure.
        id = cmd.Parameters["@ID"].Value.ToString();
        this.ClmntTbl_ID = Convert.ToInt32(id);

        connection.Close();    
        // assuming that if I open it (becuase it wasn't open already) then I should close.   This may not be correct.   Will have to investigate.
    }
    else
    {
        cmd.ExecuteNonQuery();  // If it's already open, then control for that portion of the processess is in the code calling this method so just execute the query.
        id = cmd.Parameters["@ID"].Value.ToString();
        this.ClmntTbl_ID = Convert.ToInt32(id);
    }
}
Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
MIkeN
  • 21
  • 2
  • 3
    This would be perfectly explainable if `@ID` was `NULL`. Without the definition of your table and stored procedure and the actual command you're executing through `cmd` it's not possible to say if something might be going wrong there. – Jeroen Mostert Jan 28 '20 at 15:42
  • Well .. the ID field is auto-incramented ... it's not null, you can look at the record and it gets a value. I'll try to post it. I'm admittedly new to the DBA side of things ... or at least it's been 15 years or so. – MIkeN Jan 28 '20 at 15:57
  • I didn't say the `ID` column was `NULL`, I said the `@ID` parameter might be `NULL` (the string representation of which is indeed the empty string). There are plenty of ways for that to go wrong between the sproc code and however you call it. We're missing how you create the `cmd` instance (and in particular the command text and type) and the sproc definition. – Jeroen Mostert Jan 28 '20 at 15:59
  • Adding the Method I have in my class. It's called "Add_Claimant", to the original question, towards the bottom. – MIkeN Jan 28 '20 at 16:07
  • 1
    Well there you go. If you want an output parameter in an `EXEC` statement, you have to explicitly tag it `OUTPUT` in the query text itself. This would not be necessary if you used the text `ADD_CLAIMANT_POC` and set the `CommandType` to `CommandType.StoredProcedure`, which is the recommended way to execute sprocs. This way no explicit listing of parameter names is needed. – Jeroen Mostert Jan 28 '20 at 16:10
  • 1
    Also, your code contains quite a few things to make one frown one's eyebrows. Use `using` blocks to properly dispose of commands and connections and do not reuse `SqlConnection` objects across scopes unless this is necessary for transactions (so no checking for `connection.State` is necessary in the first place). Connections are pooled; creating `SqlConnection` instances is "free" as the physical connection is kept open in the background. – Jeroen Mostert Jan 28 '20 at 16:17
  • In the param list above .... making it @ID output worked. I will investigate your alternative, seems cleaner, and less code. I've been out of modern programming for a while, so I have a lot of catch-up learning to do. Appreciate the help. – MIkeN Jan 28 '20 at 16:19

2 Answers2

1

Don't do this:

id = cmd.Parameters["@ID"].Value.ToString();
this.ClmntTbl_ID = Convert.ToInt32(id);

Because of localization/culture issues, conversions between strings and numbers or strings and dates are surprisingly expensive operations for computers. Moreover, they also tend to be sources of bugs. They're something you want to minimize and avoid when you can. The @ID parameter value already is an integer, so the final assignment can skip converting to string and back again, and look more like this:

this.ClmntTbl_ID = (int)cmd.Parameters["@ID"].Value;

But this assumes you actually have a value there. You still need to account for when @ID is NULL. This is the problem from the original code. If @ID is null, calling .ToString() produces an empty string, which matches the value you found when you saw the error message. So let's add a null check:

var result = cmd.Parameters["@ID"].Value;
if (! DBNull.Value.Equals(result))
    this.ClmntTbl_ID = (int)result;

What you want to do in case that check fails is up to you. But probably you want to fix the stored procedure to find out why it's producing NULL here in the first place.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
0

It was fixed. In my parameter list I passed to:

SqlCommand cmd = connection.CreateCommand();

cmd.CommandText = "Execute ADD_CLAIMANT_POC " + Param_String;

I should have had @ID OUTPUT instead of @ID.

There is another better way to do it, and I'll adopt that method but I've been out of modern programming for about 11 years (Been on a Mainframe), and am just learning C# to boot.

As such I'm sure I have plenty of "best practices" to learn.

Thanks to everyone who helped.

MIkeN
  • 21
  • 2
  • Yes, there is. You're already adding Parameters to the Command object, so change the .CommandType property to StoredProcedure, and then the value of CommandText becomes simply the SP name. You're kind of wasting the effort of adding the parameters to the `cmd` object if you're also concatenating a string of SQL for CommandText. – HardCode Jan 28 '20 at 17:18