Creating a factory method for creating new SqlConnection objects

Question

FOLLOW UP TO MARC GRAVELLS SUGGESTION IN THIS QUESTION

I now have something like this repeated several times in my code:

using (var conn = CreateConnection())
using (var dataCommand = conn.CreateCommand()) 
{
        conn.Open();
        [...]                        
}

Is the following correct for the factory method CreateConnection()? Or will it be error prone? (note: I'm only ever going to call it from a using directive)

SqlConnection CreateConnection()
{
    SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["IMS"].ConnectionString);
    return conn;
}

Or would there be a case for amending this method and having the Open in it as well?

Hello whytheq you want avoid redundancy code in your services ? — Aghilas Yakoub, Jul 27 '12 at 12:43
I want to avoid repeating this line `SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["IMS"].ConnectionString);` in lots of places in my code — whytheq, Jul 27 '12 at 12:47

score 5 · Accepted Answer · edited May 23 '17 at 11:56

Your code is the typical example of static factory method.

Factory Pattern is a part of Software Design Patterns, which are general reusable solutions to a commonly occurring problem within a given context in software design. I can suggest you to read Head First Design Patterns which is a very good starter book to understand design patterns.

Couple of suggestions about your code:

Make the factory method static, it doesn't use any instance variables etc.
Not related but, you don't need to use a local variable in your factory method, you can directly return the connection.

Now your method looks like this:

static SqlConnection CreateConnection(){
    return new SqlConnection(ConfigurationManager.ConnectionStrings["IMS"].ConnectionString);
}

You might wanna check if the ConnectionStrings["IMS"] is null before calling the constructor of SqlConnection. No further error handling is required in this class because it doesn't initiate the connection.

Let's say you want to return an open connection and handle connection errors in the same method:

static SqlConnection CreateConnection()
{

    if (ConfigurationManager.ConnectionStrings["IMS"] == null)
    {
        throw new Exception("Connection string not found in the configuration file.");
    }
    var sqlConnection = new SqlConnection(ConfigurationManager.ConnectionStrings["IMS"].ConnectionString);
    try
    {
        sqlConnection.Open();
    }
    catch (Exception exception)
    {
        throw new Exception("An error occured while connecting to the database. See innerException for details.", exception);
    }
    return sqlConnection;
}

If you want, you can create your own exception classes to handle these exceptions later. See ASP MVC N-Tier Exception handling. You can also return null if an exception occurs, but check "Is returning null bad design?" first.

+1 for help so far: could the check of whether ConnectionStrings["IMS"] is null be done within this statuc method? What other methods in connection with getting in touch with the server could be wrapped up in these static factory methods? — whytheq, Aug 06 '12 at 13:32
For example, you can return an open connection, in that case, you can check whether the connection is established or not (and handle connection errors) — M. Mennan Kara, Aug 06 '12 at 14:36
can you add an example, to your answer, of the syntax I'd use to achive this? — whytheq, Aug 06 '12 at 17:03

score 2 · Answer 2 · edited Dec 10 '19 at 10:00

Factory and Singleton:

My rantings. Corrections welcome!!

Functionally, I have seen a disconnect in the way developers understand factory pattern. In simple sense, a factory will and should give a product. (eg: if you have a "Car Factory" it should give a "Car").

Having said that, you implement factory pattern only when you don't want instantiation logic to be exposed and there is a need to create different concrete products. In your case , if your need is always a SQL object(one kind of product) then why Factory? You can argue about extensibility and scalability but that is subjective. Most companies just live with either SQL server or Oracle for ages and you will not use the scalability that you built in your product.

I believe, you can live well with a Singleton pattern (or) a even simpler class with static members.
Another important method to have is CloseConnection() method. In your original code, you have used using which kills and disposes the object but in your new static implementation , you may want to build it.

    public static void CloseConnection(SQLConnection conn)
    {
        if (conn.State == ConnectionState.Open)
        {
           conn.Close();
           conn.Dispose();
        }
    }

Creating a factory method for creating new SqlConnection objects

2 Answers2

Linked