"Not all code paths return value" and when to use using statement

[maurices5000]

I keep getting an error "not all code paths return a value." I've tried placing the return statement in several places to resolve the issue. Obviously I don't understand how to resolve this seemingly simple and common problem.

Also I'm using a using statement for the datareader and a try catch block for my connection object. Does this look ok? It seems a bit sloppy to use to approaches to the same thing but i'm not sure I can do my error handling with a using statement.

Here is the method.

        public DataTable GetETLClientList(string ConnectionString)
    {
        // Set up parameters in parameter array 
        SqlParameter[] arParms = new SqlParameter[0];

        //arParms[0] = new SqlParameter("@Email", SqlDbType.VarChar);
        //arParms[0].Value = Email;


        SqlTools tools = SqlTools.CreateAndConnectSqlTools(ConnectionString);

        try
        {
            SqlCommand command = new SqlCommand();
            command.CommandType = CommandType.StoredProcedure;
            command.CommandText = "GetETLClientList";

            //command.Parameters.AddRange(arParms);
            //SqlParameter param = command.Parameters.AddWithValue("@param", value);

            DataTable dtETLClient = new DataTable("ETLClient");

            using (SqlDataReader dataReader = tools.ExecuteDataReader(command))
            {
                // A simple google search will show numerous articles and messageboards explaining that a datareader should not be passed between tiers
                // While it is not a bad thing, it is not a best practice and could cause problems closing the datareader. 
                // To resolve this problem, i convert the datareader to a datatable.
                dtETLClient.Load(dataReader);
                dataReader.Close();
            }
           return dtETLClient;
        }

        catch (SqlException ReadError)
        // catch (Exception ex)
        {
            //throw;
            pErrorMessage = ReadError.Message.ToString();
            pErrorNumber = ReadError.Number;
            pErrorClass = ReadError.Class;
            pErrorState = ReadError.State;
            pErrorLineNumber = ReadError.LineNumber;

            pTransactionSuccessful = false;
        }
        finally {
            tools.Disconnect();
        }
    }

[zachattack05]

Hiya!

Your catch and/or finally blocks don't return a value. If the try statement throws an exception, and the catch block handles it, it should return a Datatable (or rethrow the exception), in either the catch or finally block.

I use the using statement when I have something that doesn't dispose of on its own. I would probably do the same thing you did otherwise I would explicity dispose of the object when I was finished with it. You can read about the using statement here.

Hope this helps!

[maurices5000]

so are you saying i need 3 return statements one for TRY, CATCH and Finally?

thanks!

[zachattack05]

I think one in the finally block will work.

But yes, a try statement is a branch in the code, things can go fine and the try and finally block execute, but if it goes bad, the catch block might fix/handle the exception and the finally block will still execute. If the exception is unhandled and there is no catch for the exception thrown, the method immediately exits and it doesn't matter if there is a return statement because it will never reach it.

Try putting your return statement in the finally block and see if that fixes it.

[maurices5000]

Thanks very much for your suggestions!
Putting the return in the Finally block did not work. However, pulling the datatable out of the try catch and defining it in the main section of the method and placing the return outside the try catch as well. that worked. Go Figure. Thanks again.

        public DataTable GetETLClientList(string ConnectionString)
        {
            // Set up parameters in parameter array 
            SqlParameter[] arParms = new SqlParameter[0];

            //arParms[0] = new SqlParameter("@Email", SqlDbType.VarChar);
            //arParms[0].Value = Email;


            SqlTools tools = SqlTools.CreateAndConnectSqlTools(ConnectionString);
            DataTable dtETLClient = new DataTable("ETLClient");

            try
            {
                SqlCommand command = new SqlCommand();
                command.CommandType = CommandType.StoredProcedure;
                command.CommandText = "GetETLClientList";            

                using (SqlDataReader dataReader = tools.ExecuteDataReader(command))
                {
                    // A simple google search will show numerous articles and messageboards explaining that a datareader should not be passed between tiers
                    // While it is not a bad thing, it is not a best practice and could cause problems closing the datareader. 
                    // To resolve this problem, i convert the datareader to a datatable.
                    dtETLClient.Load(dataReader);
                    dataReader.Close();
                }

            }

            catch (SqlException ReadError)
            {
                pErrorMessage = ReadError.Message.ToString();
                pErrorNumber = ReadError.Number;
                pErrorClass = ReadError.Class;
                pErrorState = ReadError.State;
                pErrorLineNumber = ReadError.LineNumber;

                pTransactionSuccessful = false;
            }
            finally {
                tools.Disconnect();

            }
            return dtETLClient;
        }

[zachattack05]

Hmmm...I suppose that works as well. Be careful though. If the try block fails and it is caught, chances are your dtETLClient object will be empty.

I would keep the return for the dtETLClient object in the finally block and put a return Null; after the finally block and just have the caller check for a null value. Or you could throw your own exception after the finally block. If everything goes correctly the method will return in the finally block and never reach the return or the exception after the finally block.

Just my 2¢ worth.