[newbie] function to return MAX + 1 from primary key

[jon 80]

I'm creating a function that should return the max value of a field within the database.. 1. Any idea why I'm getting the following compilation errors? 2. I need to return a value of type System.Data.Sqltypes.SqlInt64, whilst the return value is a string, any idea how to do the conversion? An explicit cast does not work i.e.

if (rawValue != DBNull.Value) {
return {System.SqlTypes.SqlInt64.rawValue.ToString();

Errors: 1. Warning 1 Unreachable code detected C:\Inetpub\wwwroot\TaskEntry\TaskEntry.aspx.cs 163 13 C:\...\TaskEntry\ 2. Error 2 'TaskEntry.nextTaskRef()': not all code paths return a value C:\Inetpub\wwwroot\TaskEntry\TaskEntry.aspx.cs 149 43 C:\...\TaskEntry\

private System.Data.SqlTypes.SqlInt64 nextTaskRef() {

    try
    {
            string getTaskRef = @"SELECT MAX(\[taskID\_PK\]) FROM \[teamwiki\].\[dbo\].\[task\];";
            SqlCommand getTaskRefSQL = new SqlCommand(getTaskRef, conAppDB);

            object rawValue = getTaskRefSQL.ExecuteScalar();
            if (rawValue != DBNull.Value) {
                return (System.Data.SqlTypes.SqlInt64) rawValue;

            }
    }
    catch (Exception ex) {
        throw ex;
        return 0; //tried to return null as well unsuccessfullly
                  //this indicates an error condition :)
    }
    finally {
        conAppDB.Close();
    }
}

Environment notes Visual Studio 2005 Team edition / .NET 2.0 ASP.NET SQL Server 2005 Win XP SP3

Jon

modified on Wednesday, January 28, 2009 10:00 AM

[SeMartens]

Yep, you are only returning a value if the rawValue is not null. Put a return null statement at the end of the method if you like. Regards Sebastian

It's not a bug, it's a feature! Check out my CodeProject article Permission-by-aspect. Me in Softwareland.

[Karmendra Suthar]

Hi,

private System.Data.SqlTypes.SqlInt64 nextTaskRef()
{
try
{
string getTaskRef = @"SELECT MAX([taskID_PK]) FROM [teamwiki].[dbo].[task];";
SqlCommand getTaskRefSQL = new SqlCommand(getTaskRef, conAppDB);

            object rawValue = getTaskRefSQL.ExecuteScalar();
            if (rawValue != DBNull.Value) {
                return (System.Data.SqlTypes.SqlInt64) rawValue; **//if you return from this
                                                                    // point your finally block will not 
                                                                    // get executed, and becomes unreachable code**
            }
    }
    catch (Exception ex) {
        throw ex; **//once you throw an exception it comes out of catch block, so following code is unreachable**
        return 0; //tried to return null as well unsuccessfullly
                  //this indicates an error condition 

    }
    finally {
        conAppDB.Close();
    }
    **return 0; // should have return here as well, at instancecs when it reaches this point
                 // before quiting it sholud return something**
}

This code can be best written as follows

private System.Data.SqlTypes.SqlInt64 nextTaskRef()
{
object rawValue;
try
{
string getTaskRef = @"SELECT MAX([taskID_PK]) FROM [teamwiki].[dbo].[task];";
SqlCommand getTaskRefSQL = new SqlCommand(getTaskRef, conAppDB);
rawValue = getTaskRefSQL.ExecuteScalar();
}
catch (Exception ex) {
throw ex;
}
finally {
conAppDB.Close();
}
if (rawValue != DBNull.Value) {
return (System.Data.SqlTypes.SqlInt64) rawValue;
}
return 0;
}

Happy Coding, Karmendra

[Wendelius]

To give a little bit different approach. Now you're using a meaningful value as a primary key. Could you consider using surrogate key[^] instead. In order to do that you would define your primary key column as IDENTITY. After that SQL Server takes care of the numbering so one less thing to worry about. Especially if you use foreign keys between tables, I really encourage you to consider this option.

The need to optimize rises from a bad design.My articles[^]

[riced]

The finally block will always be executed so your comment against the if statement is wrong. The purpose of the finally block is to ensure that code is run no matter how the try block is exited even if an exception is thrown. Your comment in the catch block is spot on - the return is unreachable and completely redundant. However your solution is good. An alternative would be to add an else statement that returns 0 after the if. I.e do this

if (rawValue != DBNull.Value) {
return (System.Data.SqlTypes.SqlInt64) rawValue;
}
else {
return 0;
}

Regards David

[jon 80]

Thanks, I was having issues with explicit casting the value to SqlInt64:

return (System.Data.SqlTypes.SqlInt64) rawValue;

Jon