0
\$\begingroup\$

I'm using a database that has a library for access to it. Connection is done synchronously, it's blocking and I do not have a way to abort it. To connect, I have to write:

var _loader = new DatabaseDriverLoader();
DatabaseDriver _driver = _loader.Connect(connectionString, login, password);

I wanted to improve it:

  • Be able to connect asynchronously (not waiting for actual connection)
  • Be able to cancel the connection if I need to

Here's a wrapper that I created:

public class DatabaseConnection : IDisposable
{
    private DatabaseDriver _driver;
    private DatabaseDriverLoader _loader

    public event EventHandler ConnectionFailed;
    public event EventHandler ConnectionEstablished;
    public event EventHandler ConnectionLost;

    public async Task ConnectAsync(string connectionString, string login, string password, CancellationToken cancellationToken)
    {
        var connectionTask = Task.Run(() =>
        {
            try
            {
                var _loader = new DatabaseDriverLoader();
                DatabaseDriver _driver = _loader.Connect(connectionString, login, password);
            }
            catch (Exception e) { }
        });

        await Task.Run(async () =>
        {
            while(!connectionTask.IsCompleted && !cancellationToken.IsCancellationRequested)
            {
                await Task.Delay(100);
            }
        });

        cancellationToken.ThrowIfCancellationRequested();

        if (_driver == null)
        {
            ConnectionFailed?.Invoke(this, EventArgs.Empty);
            return;
        }
        ConnectionEstablished?.Invoke(this, EventArgs.Empty);

        _driver.ConnectionToServerLost += OnConnectionLost;
    }

    public void Dispose()
    {
        if (_driver != null)
        {
            _driver.ConnectionToServerLost -= OnConnectionLost;
            _driver.Dispose();
        }
        _loader?.Dispose();
    }

    public DatabaseDriver GetDriver()
    {
        return _driver;
    }

    private void OnConnectionLost(object sender, EventArgs e) => ConnectionLost?.Invoke(sender, e);
}

Here's how I would use that class:

private async Task ConnectWithDatabase() 
 {
  //Delete old connection instance
  _dbConnectionTokenSource?.Cancel();
  if (_dbConnection != null)
  {
      _dbConnection.ConnectionEstablished -= OnDbConnectionEstablished;
      _dbConnection.ConnectionLost -= OnDbConnectionLost;
      _dbConnection.ConnectionFailed -= OnDbConnectionFailed;

      _dbConnection.Dispose();
  }

//Create new connection instance
  _dbConnection = new DatabaseConnection();
  _dbConnection.ConnectionEstablished += OnDbConnectionEstablished;
  _dbConnection.ConnectionLost += OnDbConnectionLost;
  _dbConnection.ConnectionFailed += OnDbConnectionFailed;

  _dbConnectionTokenSource = new CancellationTokenSource();
  var token = _dbConnectionTokenSource.Token;

  try
  {
      await _dbConnection.ConnectAsync(
          ConnectionString,
          Login,
          Password,
          token)
      .ConfigureAwait(false);
  }
  catch (TaskCanceledException e)
  { }
 } 

I wanted to ask you kindly for review of this code, because I'm not really sure if this is done correctly. I don't know if it is a good idea to use an event for successful connection. Maybe Connect method should return successful Task when connection occurs?

I'm not sure if cancellation is done correctly. In my use case there might be situations where DatabaseConection is canceled and disposed before connection occurs successfully.

I am not sure if it's a good idea to create new threads in DatabaseConnection, because clients of this class might not want it? - at least I read somewhere that it's not a good design, but I'm not sure why.

I will be happy to hear any opinion, always good to learn something new.

\$\endgroup\$
  • \$\begingroup\$ @HenrikHansen Ofcourse, you are right. Mistakes came over, because I modified code a bit before posting here to remove unnecessary parts. I fixed it now. Async behaviour is needed, because I do not want thread to be blocked when I start connecting to database. \$\endgroup\$ – Loreno Mar 7 at 7:30
  • 1
    \$\begingroup\$ because I modified code a bit before posting here to remove unnecessary parts - there are no unnecessary parts on Code Review. Please post your actual code without modifying it. Otherwise you might not get the feedback you are hoping for and both reviewers and you will be unhappy about having wasted their time. \$\endgroup\$ – t3chb0t Mar 7 at 7:37
  • \$\begingroup\$ @t3chb0t Modifications were about names and very small refactoring. After fixing, my code is same as I posted here. \$\endgroup\$ – Loreno Mar 7 at 10:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.