23

I found this piece of code in one application

Database database = DatabaseFactory.CreateDatabase("connection string");
DbConnection connection = database.CreateConnection();
connection.Open();
SqlConnection sqlConnection = (SqlConnection)connection;

Is it safe, SqlConnection derieve from DbConnection. Database comes from Microsoft.Practices.EnterpriseLibrary.Data. According to documentation CreteDatabase returns DbConnection.

Darqer
  • 2,847
  • 9
  • 45
  • 65

4 Answers4

17

No it is not safe, casting is never safe and it may blow anytime while your application is running. While SqlConnection derives indeed from DbConnection you are not guaranteed that database.CreateConnection() will return a SqlConnection as this could be parametrized in the configuration file. Also why do you need to cast to SqlConnection? It is always better to work with classes that are higher in the hierarchy to avoid coupling your code with a specific implementation which will make your code impossible to test in isolation.

While the EnterpriseLibrary does a decently good job in keeping things abstract you are killing everything with this cast. Also you should make sure that disposable resources are always disposed properly. How about this instead:

Database database = DatabaseFactory.CreateDatabase("connection string");
using (var conn = database.CreateConnection())
using (var cmd = conn.CreateCommand())
{
    conn.Open();
    cmd.CommandText = "SELECT id FROM foo";
    using (var reader = cmd.ExecuteReader())
    {
        while (reader.Read())
        {
            // TODO: work with the results here
        }
    }
}

This way your code is less fragile to database changes in the config file. Well of course you still have this SQL hardcoded and there are ORMs that will take care of this situation. They will also allow you to focus on the real domain of your application instead of wasting time in writing SQL queries and casting from one database provider to another. But for a simple application this is OK.

Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
11

It should be safe as long as you never change the connection string to connect to anything other than a SQL Server Database. If that's ever a possibility, then you should add a little more logic to make things safe:

Database database = DatabaseFactory.CreateDatabase("conn string");

using(DbConnection conn = database.CreateConnection())
{    
    if(conn is SqlConnection)
    {
        var sqlConn = conn as SqlConnection;
    }
}
Justin Niessner
  • 242,243
  • 40
  • 408
  • 536
6

It depends on the databases you are using in your application. From the code you wrote it looks like that only SQL Server is used. If it is so, then you can cast DbConnection to SqlConnection safely. In fact DbConnection is a base class for any other database connection. In your case it is SqlConnection (which is used to work with SQL Server database), also there are different databases like Oracle, Mysql, etc and their providers usually have own classes for connections. So if your app uses another databases or may use in the future it is unsafe to have such cast.

Andrew Bezzub
  • 15,744
  • 7
  • 51
  • 73
  • 2
    it not only depends on the database used but more directly it depends on the type the factory returns based on the database in use. If they ever decided to create a new connection class working with SQL server the code could fail – Rune FS Sep 05 '10 at 20:09
0

You can always do a check and convert it to SqlConnection with C# pattern matching (C# 7.0+)

Database database = DatabaseFactory.CreateDatabase("conn string");

using(DbConnection connection = database.CreateConnection())
{    
    if(connection is SqlConnection sqlConnection)
    {
        // do something with sqlConnection
    }
    else
    {
       throw new InvalidOperationException("Connection is not to a SQL Database");
    }
}
Todd Skelton
  • 6,839
  • 3
  • 36
  • 48