Proper use of Singleton in DAL and BO layer?

Question

I'm tasked with implementing a Business Object / Data Access Layer for a project and have to expect thousands of users concurrently. I've always used singletons to manage the DAL but I never game too much thought about how it would behave with so many multiple users at the same time, so I'd like to ask the proper use for it.

I have:

public class UserDAL
{
    private static UserDAL _userDAL = null;

    //Private constructor 
    private UserDAL() { }

    public static UserDAL GetInstance()
    {
        if(_userDAL == null)
        { _userDAL = new UserDAL(); }

        return _userDAL;
    }

    //Example of a method
    public User GetUsers()
    {
        IDataReader dataReader = ConnectionFactory.GetConnection().ExecuteSomeQuery("queryHere");
    }
}

For my Connection Factory I don't think it's a problem, although I did read that it's best to leave the connection pooling to ADO.NET itself:

public sealed class ConnectionFactory
{
    private static string _connectionString = ConfigurationManager.ConnectionStrings["ConnectionName"].ConnectionString;

    //My connection interface
    private static IConnection _connection = null;

    public static IConnection GetConnection()
    {
        if(_connection == null)
        {
            //some checks to determine the type
            _connection = new SQLConnection(_connectionString);
        }
        return _connection;
    }
}

I'm also using the singleton pattern in the BO, although I don't think it's necessary:

public class UserBO
{
    private static UserBO _userBO = null;
    private static UserDAL _userDAL = null;

    private UserBO() { }

    public static UserBO GetInstance()
    {
        if(_userBO == null)
        {
            _userBO = new UserBO();
            _userDAL = UserDAL.GetInstance();
        }

        return _userDAL;
    }

    //Example of a method
    public User GetUser()
    {
        //Rules
        return _userDAL.GetUsers();
        //return UserDAL.GetInstance().GetUsers(); //or this
    }
}

I'm doing it like this just so I can call in the UI/Presentation layer:

User someUser = UserBO.GetInstance().GetUser(1);

This worked for me for the applications I've made so far, but I'm guessing it's because there wasn't too many users simultaneously. I'm worried about what would happen in the UserDAL instance when a second user requests something but there's already a 1st user doing some heavy operation in it.

Should I drop this pattern in the BO/DAL layer and leave it only in the ConnectionFactory? Are there any issues which I should expect if I use this?

Answer 1

I would definitely drop it altogether, especially for the Connection: the connectionFactory could be static, but return a new connection each time it is asked: ADO.NET is very good at managing connection pooling and you just need to get out of it's way.

In anything which has changeable state keep away from singletons. This includes ADO.NET connections, and your actual Business Objects. Having one user mutate the state of an object that is being used by another user can lead to all sorts of strange bugs: in a web site, you basically have a massively multithreaded application and changeable singletons are very bad news!

You do need to come up with some sort of locking strategy, though, for when two or more users change copies of the same business object. A valid strategy includes saying 'Actually, this isn't going to be a problem so I'll ignore it' - but only if you have thought about it. The two basic strategies are Optimistic and Pessimistic Locking.

Optimistic Locking means that you optimistically think mostly the users won't change the same things (for whatever reason) and so you don't put Database locks on read data. This is the only possibility on a Web Site

Pessimistic locking says all possibly changed data will, when read, have DB Locks applied until the user is finished with it. This means keeping a Transaction open, and it's not practical for a Web Site.

Optimistic Locking can be implemented by creating Update Statements which update a row only where all columns which haven't been changed by the current user also haven't been changed in the database; if they have, someone else has changed the same row. Alternatively, you can add a column to all tables - version int not null - and update where the version hasn't changed since you read the object; you also increment the version number in every update.

If either method fails, you need to reread the now-current data and get your user to confirm or re-apply their changes. Bit of a pain but can be necessary.

Answer 2

I would advise you to move away from the Singleton pattern for testability: Dependency Injection & Singleton Design pattern

Instead, take a look at Dependency Injection. Ninject is a good way to start.

DI will take care of wiring the BO and DAL together:

public interface IUserRepository
{
     IEnumerable<User> GetUsers();
}

public class UserBO
{
     private readonly IUserRepository _userRepository;

     public UserBO(IUserRepository userRepository){
         _userRepository = userRepository;
     }

     public IEnumerable<User> GetUsers()
     {
         return _userRepository.GetUsers();
     }
}

As for reusing the Connection Pool: Should you reuse SqlConnection, SqlDataAdapter, and SqlCommand objects?