Correct use of Try Catch for the SQL connection in C#

Question

Is this code correct in means of Try/Catch? I need to value whether the action is done or not to inform the user about the result, so I will then get the bool value to know if connection was successful.

    public static bool CreateSQLDatabaseTable()
    {
        var connString = "Server=localhost\\SQLEXPRESS;Integrated Security = SSPI; database = MyDB";
        string cmdText = "SELECT count(*) as Exist from INFORMATION_SCHEMA.TABLES where table_name =@Product";
        try
        {
            using (var sqlConnection = new SqlConnection(connString))
            {
                using (var sqlCmd = new SqlCommand(cmdText, sqlConnection))
                {
                    sqlCmd.Parameters.Add("@Product", System.Data.SqlDbType.NVarChar).Value = "Product";
                    sqlConnection.Open();
                    sqlCmd.ExecuteScalar();
                    if ((int)sqlCmd.ExecuteScalar() != 1)
                    {
                        using (SqlCommand command = new SqlCommand("CREATE TABLE Product (Id INT, UserId TEXT, CreationDate TEXT, Name TEXT)", sqlConnection))
                        {
                            command.ExecuteNonQuery();
                            return true;
                        }
                    }
                }

            }
            return false;
        }
        catch
        {
            return false;
        }
    }

the catch should actually catch a possibly thrown exception (and do something about it, not just swallow it) — Jeff, May 18 '18 at 22:01
after this method call is made I will check for true/false and respond to user. Is that ok? — positive perspective, May 18 '18 at 22:04
Also don't use `TEXT` data type for SQL columns it is deprecated. Use `VARCHAR` / `VARCHAR( MAX )` — Alex, May 18 '18 at 22:24
if you call this method and it returns false, how do you know if the table already exists or if there was an exception thrown? — Rufus L, May 18 '18 at 23:34
@Alex: Better NVARCHAR / NVARCHAR(MAX), then he never has an issue with internationalization. — Christoph, Jul 03 '20 at 07:23
The query to determine whether the table exists and after the if-check are imperfect: If you have a table "OtherSchema.Product", your table "dbo.Product" is never created. If you have a table "OtherSchema.Product" and a table "dbo.Product", your query crashes because it attemps to create table "dbo.Product" again. — Christoph, Jul 03 '20 at 07:26

score 3 · Accepted Answer · answered May 18 '18 at 22:22

Your method can actually have 3 outcomes:

The table was created successfully (method returns true)
The table already exists (method returns false)
There was an error trying to create the table (exception is thrown)

So you should handle the exception outside of this method and you should not blindly catch all Exceptions. You should only handle the SqlException so that other exceptions will not be handled. Also you should be logging the exception somewhere as a good practice.

For more information on the SqlException
https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlexception(v=vs.110).aspx

public static bool CreateSQLDatabaseTable()
{
    var connString = "Server=localhost\\SQLEXPRESS;Integrated Security = SSPI; database = MyDB";
    string cmdText = "SELECT count(*) as Exist from INFORMATION_SCHEMA.TABLES where table_name =@Product";

    using (var sqlConnection = new SqlConnection(connString))
    {
        using (var sqlCmd = new SqlCommand(cmdText, sqlConnection))
        {
            sqlCmd.Parameters.Add("@Product", System.Data.SqlDbType.NVarChar).Value = "Product";
            sqlConnection.Open();
            sqlCmd.ExecuteScalar();
            if ((int)sqlCmd.ExecuteScalar() != 1)
            {
                using (SqlCommand command = new SqlCommand("CREATE TABLE Product (Id INT, UserId TEXT, CreationDate TEXT, Name TEXT)", sqlConnection))
                {
                    command.ExecuteNonQuery();
                    return true;
                }
            }
        }
    }

    return false;
}

public static void Main()
{
    try
    {
        bool wasCreated = CreateSQLDatabaseTable();
    }
    catch (SqlException ex)
    {
        // Handle the SQL Exception as you wish
        Console.WriteLine(ex.ToString());
    }
}

What is the argument for moving the try catch outside of the method itself and to the caller of the method instead? — Rufus L, May 18 '18 at 22:33
It could technically be inside the method, but this gives the flexibility of handling all 3 outcomes listed above. In @positive-perspective initial example he returns false in both cases of an error or if the table already exists. I don't think this was the intended implementation. If the try catch moves inside the method false should only be returned in the case of an error, otherwise it should always return true. This gives less flexibility. — Roland Nasr, May 18 '18 at 22:39

score 0 · Answer 2 · answered Jul 03 '20 at 08:03

I would solve such a problem through a single query that does the exists check itself and returns whether it was created or not.

    public static bool CreateSQLDatabaseTable() {
        var connString = "...";
        const string tableName = "Product";
        string cmdText = $"IF EXISTS (SELECT TOP(1) 1 FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_CATALOG = DB_NAME() AND TABLE_SCHEMA = 'dbo' AND TABLE_NAME = '{tableName}' AND TABLE_TYPE = 'BASE TABLE') BEGIN\r\n" +
                         $"  SELECT CAST(0 AS BIT) AS CreatedNow\r\n" +
                         $"END ELSE BEGIN\r\n" +
                         $"  CREATE TABLE dbo.{tableName}(Id INT, CreatorId INT, CreationDate DATETIME, CreationDateUtc DATETIME, Name NVARCHAR)\r\n" +
                         $"  SELECT CAST(1 AS BIT) AS CreatedNow\r\n" +
                         $"END";

        using (var sqlConnection = new SqlConnection(connString)) {
            using (var sqlCmd = new SqlCommand(cmdText, sqlConnection)) {
                sqlConnection.Open();
                return (bool)sqlCmd.ExecuteScalar();
            }
        }
    }

Correct use of Try Catch for the SQL connection in C#

2 Answers2

Linked