current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 11 down vote favorite
2

I've created a small method to test the connection to a database:

public bool TestConnection(string provider, string serverName, string initialCatalog, string userId, string password, bool integratedSecurity)
{
    var canConnect = false;

    var connectionString = integratedSecurity ? string.Format("Provider={0};Data Source={1};Initial Catalog={2};Integrated Security=SSPI;", provider, serverName, initialCatalog) 
                                              : string.Format("Provider={0};Data Source={1};Initial Catalog={2};User ID={3};Password={4};", provider, serverName, initialCatalog, userId, password);

    var connection = new OleDbConnection(connectionString);

    try
    {
        using (connection)
        {
            connection.Open();

            canConnect = true;
        }
    }
    catch (Exception exception)
    {

    }
    finally
    {
        connection.Close();                
    }                                         

    return canConnect;
}

Despite the method works it doesn t seems right to me. Is any way to test the connection without having to catch the exception? Is it possible to achieve the same result in a different way?

share|improve this question

migrated from stackoverflow.com Nov 9 '11 at 23:02

This question came from our site for professional and enthusiast programmers.
1  
You should use OleDbConnectionStringBuilder – SLaks Nov 9 '11 at 22:57
    
seems like a reasonable use of exceptions as logic to me, you can omit the catch since its empty and just have a try/finally block – Aaron Anodide Nov 9 '11 at 22:58
3  
@Gabriel: Wrong. – SLaks Nov 9 '11 at 23:00
    
@Slaksi know! thanks - I took your suggestion and what was left of mine (taking away the unused parameter to the catch) and made an answer.. – Aaron Anodide Nov 9 '11 at 23:14
2  
If you are going to use strings in complex scenarios such as that, I would suggest storing them in constants to make them more easily readable. – Samuel Slade Nov 10 '11 at 8:29
up vote 12 down vote

It's a little verbose.

I would write

try {
    using(var connection = new OleDbConnection(...)) {
        connection.Open();
        return true;
    }
} catch {
    return false;
}

You should also use OleDbConnectionStringBuilder to properly escape the variables in the connection string.

share|improve this answer
1  
Is there a more specific exception that should be getting caught here? – Winston Ewert Nov 9 '11 at 23:02
3  
@WinstonEwert: Good question. In principal, yes (OleDbException or DataException), but I'm not sure if I would rely on all providers conforming to that. – SLaks Nov 9 '11 at 23:03
    
You definitely should NOT be catching all exceptions. Any good provider (like the ones in the framework) will document the exceptions they through. The SQL provider throws just SQLExceptions if its a SQL specific problem. (I tried to find a good reference for my assertion but could not in a quick search ;-) – Tom Winter Nov 11 '11 at 15:01
    
@TomWinter: The Framework providers certainly will, but others may not, and unmanaged OleDEb drivers may have their own issues. – SLaks Nov 11 '11 at 15:12
up vote 2 down vote

I wrote one a little ways back that worked with SQL Server and your current user identity:

namespace DatabaseConnectionTester
{
    using System;
    using System.Data.Common;
    using System.Data.SqlClient;

    internal static class Program
    {
        private static int Main(string[] args)
        {
            bool result;

            if (args.Length == 0)
            {
                result = true;
            }
            else
            {
                try
                {
                    var connectionString =
                        "Connect Timeout=10;Pooling=false;Integrated Security=sspi;server=" + args[0];

                    using (var connection = new SqlConnection(connectionString))
                    {
                        connection.Open();
                        result = true;
                    }
                }
                catch (DbException)
                {
                    result = false;
                }

                if (args.Length > 1)
                {
                    Console.WriteLine(result);
                }
            }

            return Convert.ToInt32(result);
        }
    }
}
share|improve this answer
up vote 1 down vote

You could refactor it to a TryOpenConnection method, which would be more a more commonly accepted reason to mix logic and error handling (which otherwise is a bad idea). I would think you actually want to use this connection? You don't have to open/close just for confirmation. I also think its a bit verbose to pass in each part of the connection string seperately, but you get the point: 

    public bool TryOpenConnection(string connectionString, out OleDbConnection connection)
    {
        try {     
            var conn = new OleDbConnection(connectionString); 
            conn.Open();
            connection = conn;
            return true; 
        } 
        catch (OleDbException exception) {
            connection = null; 
            return false; 
        } 
    }
share|improve this answer
2  
Good idea ...except that you should use the using clause. Set connection to null before try-catch; then return connection != null – IAbstract Nov 10 '11 at 2:18
    
@ANeves: No...conn.Open() is still in a try-catch. Without proper formatting the following may be a little difficult to read, but I think you will see what I'm talking about :) ... connection = null; try { using( var conn = new OleDbConnection(connectionString)) { conn.Open(); connection = conn; } } catch (OleDbException exception) { // log failure } return connection != null – IAbstract Nov 10 '11 at 14:33
up vote 1 down vote 
void Main()
{
    TestConnection(
        "myprovider",
        "myserver",
        "myinitcat",
        "theuser",
        "thepass",
        true);

    TestConnection(
        "myprovider",
        "myserver",
        "myinitcat",
        "theuser",
        "thepass",
        false);
}
public bool TestConnection(string provider, string serverName, string initialCatalog, string userId, string password, bool integratedSecurity) 
{ 
    var canConnect = false; 
    var  csb = new OleDbConnectionStringBuilder();
    csb.Provider = provider;
    csb.DataSource = serverName;
    csb.Add("Initial Calalog", initialCatalog);
    if(integratedSecurity)
    {
        csb.Add("Integrated Security", "SSPI");
    }
    else
    {
        csb.Add("User", userId);
        csb.Add("Password", password);
    }  
    var connection = new OleDbConnection(csb.ToString()); 
    try 
    { 
        using (connection) 
        { 
            connection.Open(); 
            canConnect = true; 
        } 
    } 
    catch
    {
    }
    finally 
    { 
        connection.Close();                 
    }
    Console.WriteLine (csb.ToString());
    return canConnect; 
}

Result:

Provider=myprovider;Data Source=myserver;Initial Calalog=myinitcat;Integrated Security=SSPI
Provider=myprovider;Data Source=myserver;Initial Calalog=myinitcat;User=theuser;Password=thepass
share|improve this answer
    
Use an object initializer: msdn.microsoft.com/en-us/library/bb384062.aspx var csb = new OleDbConnectionStringBuilder { Provider = provider, DataSource = serverName }; – ANeves Nov 10 '11 at 13:35

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.