try-catch statement and database connection for search query

Question

These codes work fine but I still want to know if I need to use a try-catch statement and open-close connection of database for the search query.

Your suggestions will be highly appreciated.

SqlConnection cn = new SqlConnection("Data Source = localhost; Integrated Security = True; Database = myDB;");
SqlDataAdapter adp = new SqlDataAdapter();

private void LoadSearch()
        {            
            switch (cmbCategory.Text)
            {
                case "All":                    
                    adp.SelectCommand = new SqlCommand("SELECT * FROM tblCommunication WHERE LetterType LIKE '" + txSearch.Text.Trim() + "' OR LetterNumber LIKE '" + txSearch.Text.Trim() + "' OR LetterAmount LIKE '" + txSearch.Text.Trim() + "' OR LetterFrom LIKE '" + txSearch.Text.Trim() + "' OR LetterTo LIKE '" + txSearch.Text.Trim() + "' OR ReceivedBy LIKE '" + txSearch.Text.Trim() + "' OR Requisition LIKE '" + txSearch.Text.Trim() + "' OR LetterSubject LIKE '" + txSearch.Text.Trim() + "' OR LetterContent LIKE '" + txSearch.Text.Trim() + "' OR LetterRemarks LIKE '" + txSearch.Text.Trim() + "'", cn);
                    DataTable dtAll = new DataTable();
                    //cn.Open();
                    adp.Fill(dtAll);
                    dgCommunications.DataSource = dtAll;
                    //cn.Close();                    
                    break;

                case "Incoming Communications":
                    adp.SelectCommand = new SqlCommand("SELECT CommType = '" + cmbCategory.Text + "', LetterDate, LetterReceived, LetterType, LetterNumber, LetterAmount, LetterFrom, LetterTo, ReceivedBy, Requisition, LetterSubject, LetterContent, LetterRemarks FROM tblCommunication WHERE LetterType LIKE '" + txSearch.Text.Trim() + "' OR LetterNumber LIKE '" + txSearch.Text.Trim() + "' OR LetterAmount LIKE '" + txSearch.Text.Trim() + "' OR LetterFrom LIKE '" + txSearch.Text.Trim() + "' OR LetterTo LIKE '" + txSearch.Text.Trim() + "' OR ReceivedBy LIKE '" + txSearch.Text.Trim() + "' OR Requisition LIKE '" + txSearch.Text.Trim() + "' OR LetterSubject LIKE '" + txSearch.Text.Trim() + "' OR LetterContent LIKE '" + txSearch.Text.Trim() + "' OR LetterRemarks LIKE '" + txSearch.Text.Trim() + "'", cn);
                    DataTable dtInc = new DataTable();
                   // cn.Open();
                    adp.Fill(dtInc);
                    dgCommunications.DataSource = dtInc;
                    //cn.Close();
                    break;

                case "Inside Communications":
                    adp.SelectCommand = new SqlCommand("SELECT CommType = '" + cmbCategory.Text + "', LetterDate, LetterReceived, LetterType, LetterNumber, LetterAmount, LetterFrom, LetterTo, ReceivedBy, Requisition, LetterSubject, LetterContent, LetterRemarks FROM tblCommunication WHERE LetterType LIKE '" + txSearch.Text.Trim() + "' OR LetterNumber LIKE '" + txSearch.Text.Trim() + "' OR LetterAmount LIKE '" + txSearch.Text.Trim() + "' OR LetterFrom LIKE '" + txSearch.Text.Trim() + "' OR LetterTo LIKE '" + txSearch.Text.Trim() + "' OR ReceivedBy LIKE '" + txSearch.Text.Trim() + "' OR Requisition LIKE '" + txSearch.Text.Trim() + "' OR LetterSubject LIKE '" + txSearch.Text.Trim() + "' OR LetterContent LIKE '" + txSearch.Text.Trim() + "' OR LetterRemarks LIKE '" + txSearch.Text.Trim() + "'", cn); ;
                    DataTable dtIns = new DataTable();
                    //cn.Open();
                    adp.Fill(dtIns);
                    dgCommunications.DataSource = dtIns;
                    //cn.Close();
                    break;

                case "Outgoing Communications":
                    adp.SelectCommand = new SqlCommand("SELECT CommType = '" + cmbCategory.Text + "', LetterDate, LetterReceived, LetterType, LetterNumber, LetterAmount, LetterFrom, LetterTo, ReceivedBy, Requisition, LetterSubject, LetterContent, LetterRemarks FROM tblCommunication WHERE LetterType LIKE '" + txSearch.Text.Trim() + "' OR LetterNumber LIKE '" + txSearch.Text.Trim() + "' OR LetterAmount LIKE '" + txSearch.Text.Trim() + "' OR LetterFrom LIKE '" + txSearch.Text.Trim() + "' OR LetterTo LIKE '" + txSearch.Text.Trim() + "' OR ReceivedBy LIKE '" + txSearch.Text.Trim() + "' OR Requisition LIKE '" + txSearch.Text.Trim() + "' OR LetterSubject LIKE '" + txSearch.Text.Trim() + "' OR LetterContent LIKE '" + txSearch.Text.Trim() + "' OR LetterRemarks LIKE '" + txSearch.Text.Trim() + "'", cn); ;
                    DataTable dtOut = new DataTable();
                    //cn.Open();
                    adp.Fill(dtOut);
                    dgCommunications.DataSource = dtOut;
                   // cn.Close();
                    break;                
            }            
        }

`using` would be something you need. https://stackoverflow.com/questions/212198/what-is-the-c-sharp-using-block-and-why-should-i-use-it — Prajwal, Jan 24 '18 at 04:46
one more with similar question https://stackoverflow.com/questions/12033998/is-there-any-need-to-close-a-dbconnection-if-a-using-clause-is-used — Prajwal, Jan 24 '18 at 04:51

Dah Sra · Accepted Answer · 2018-01-24T05:17:18.783

One of the best way is by using statement

using (SqlConnection connection = new SqlConnection(conString))
{
    try
    {
        //your switch case statement
    }
    catch (InvalidOperationException)
    {

    }
    catch (SqlException)
    {

    }
}

one way , You can write a comman method for all the cases like below and call this method in all the cases

public void Command(string query,string conString)
{
    SqlDataAdapter adp = new SqlDataAdapter();
    using (SqlConnection connection = new SqlConnection(conString))
    {

        try
        {
            adp.SelectCommand = new SqlCommand(query,connection);
            DataTable dtOut = new DataTable();
            adp.Fill(dtOut);
            dgCommunications.DataSource = dtOut;
        }
        catch (InvalidOperationException)
        {

        }
        catch (SqlException)
        {

        }
    }
}

or can use like this

SqlDataAdapter adp = new SqlDataAdapter();
using (SqlConnection connection = new SqlConnection(conString))
{

    try
    {
        switch (cmbCategory.Text)
        {
            case "All":                    
                break;

        }
    }
    catch (InvalidOperationException)
    {

    }
    catch (SqlException)
    {

    }
}

Do you mean for each case I should use a **using statement** ? — Juan Zero, Jan 24 '18 at 05:11
Depends if you dont want to use it then use your switch under the using statement.Updated the answer — Dah Sra, Jan 24 '18 at 05:15

score 0 · Answer 2 · answered Jan 24 '18 at 05:49

The code presented in the question is a prime example of how not to write code.

Using string concatenation to create SQL statements is a security risk - as it's an open door for SQL injection attacks. Always use parameters when passing user input to the database.
Most of the code is repeating itself. In fact, the only thing that's different in the switch branches are the SQL statements.
Always use the using statement when dealing with instances of classes that implements the IDisposable interface - and All ADO.Net classes are IDisposable. This will ensure your instances are disposed.
The use of "magic strings" in your case statement. A better solution would be to use a designated class with constants instead of using magic strings all over the code.
Your code involves both UI elements and Database access - there is no separation of concerns here.

So what should you write instead?

Well, you should be working with some kind of n-tier architecture design pattern - MVC, MVP and MVVM are some of the common implementations of n-tier.

This is way too broad to explain fully in an SO post, but basically what it means is that your code should be divided into (at least) 3 distinct parts: A data access layer, a business logic layer, and a UI (user interface) layer.

The simplest form of 3-tier would be to have one class for data access, one class for every form/window's UI, and one class for every form/window's business logic.

So for the data access, have a class designed to communicate with your database.
This class should know nothing about the UI, and nothing about the business logic. It only needs to have methods for CRUD - Create, Read, Update and Delete data.

Based on your code above, you could use something like this:

class DAL
{
    private readonly string _connectionString;
    public DAL(string connectionString)
    {
        _connectionString = connectionString;
    }

    public DataTable SearchAll(string searchText)
    {
        var sql = "SELECT * FROM tblCommunication "+
                  "WHERE LetterType = @searchText "+
                  "OR LetterNumber = @searchText  "+
                  "OR LetterAmount = @searchText  "+
                  "OR LetterFrom  = @searchText  "+
                  "OR LetterTo  = @searchText  "+
                  "OR ReceivedBy  = @searchText  "+
                  "OR Requisition  = @searchText  "+
                  "OR LetterSubject  = @searchText  "+
                  "OR LetterContent  = @searchText  "+
                  "OR LetterRemarks  = @searchText";

        var parameter = new SqlParameter("@searchText", SqlDbType.VarChar);
        parameter.Value = searchText;
        return GetDataTable(sql, CommandType.Text, parameter);
    }

    public DataTable SearchIncomingCommunications(string caterogy, string searchText) 
    { 
        // implementation same as the example above
        throw new NotImplementedException();
    }

    public DataTable SearchInsideCommunications(string caterogy, string searchText)
    {
        // implementation same as the example above
        throw new NotImplementedException();
    }

    // add other methods as well

 private DataTable GetDataTable(string sql, CommandType commandType, params SqlParameter[] parameters)
    {
        var dt = new DataTable();
        using (var con = new SqlConnection(_connectionString))
        {
            using (var cmd = new SqlCommand(sql, con))
            {
                cmd.CommandType = commandType;
                foreach (var parameter in parameters)
                {
                    cmd.Parameters.Add(parameter);
                }
                using (var da = new SqlDataAdapter(cmd))
                {
                    da.Fill(dt);
                }
            }
        }
        return dt;
    }
}

Then, in your business logic class, you put your switch statement to determine, based on input you got from the UI, what data table to return. Finally, in the UI, you link the DataTable to your dgCommunications data source.

As for the error handling - Well, This doesn't belong in the data access layer. It belongs in the business logic layer - since in the data access layer there is little you can do about it. Read this question on softwareengineering.stackexchange.com for more details.

I would have explain further but I suspect I've already gone beyond the scope of a single Stack Overflow answer. The rest if left for you to research. A good place to start would be This SO question and it's answers.

Thank you for the answers and suggestions, I really appreciate it. — Juan Zero, Jan 24 '18 at 10:58

try-catch statement and database connection for search query

2 Answers2