0

I just have a question related to "best practice". If this question isn't constructive, then just vote it down :)

I had a discussion with a colleague some days ago, we have two completely different philosophies when it comes to best practice regarding open connections to a web service, COM-object, database etc.

I prefer wrapping the connection code in a class that implements IDisposable and let that handle all that comes to connection etc. A short example.

    public class APIWrapper : IDiposable
    {
          public APIWrapper(...)
          {
              DataHolder = new DataHolder(...);

              /// Other init methods

              Connect();
          }

          public [Webservice/Database/COM/etc.] DataHolder { get; private set; }

          public void Connect()
          {
              DataHolder.Connect(...);
          }

          public void Disconnect()
          {
              DateHolder.Disconnect();
          }

          public void Dispose()
          {
              Dispose(true);
              GC.SuppressFinalize(this);
          }

          private void Dispose(bool disposing)
          {
              if(disposing)
              {
                  if (DataHolder != null)
                  {
                      Disconnect();
                      DataHolder = null;
                  }
              }
          }
    }

And I will use it like this in my Data controller.

    public class UserController
    {
          ....

          public IList<User> getUsers()
          {
              using(APIWrapper datalayer = new APIWrapper(...))
              {
                  var users = datalayer.DataHolder.GetUsers();

                  /// Map users to my enity of users
                  return mappedUsers;
              }
          }
    }

And my colleagues would look like this:

    public class UserController
    {

         protected [Webservice/Database/COM/etc.] DataHolder { get; set; }

          public UserController()
          {
              DataHolder = new DataHolder(...);
              DataHolder.Connect(...);
          }

          public IList<User> getUsers()
          {
              var users = DataHolder.GetUsers();

              /// Map users to my enity of users
              return mappedUsers;
          }

          /// Who should call this?
          public void Dispose()
          {
              DataHolder.Disconnect();
          }
    }

The code above are just examples that are simplified (and written i stackoverflow text editor), but I think they show the necessary philosophies.

So, the first example will open and close to connection at each call. The second example will hold the connection open for a longer amount of time.

What is "generally" best practice in your opinion?

Avinash Jain
  • 7,200
  • 2
  • 26
  • 40
Alex
  • 487
  • 1
  • 6
  • 19
  • 1
    I think it would really depend on the situation and what the end point actually is that you're talking to. SQL server, for instance, recommends the first approach. However if you're dealing with something that takes a long time to set up/tear down the connection, there would be an argument for the second approach. – James Thorpe Nov 24 '15 at 09:52
  • @PatrickHofman it might work on code review as long as OP provides his actual code and not examples. – Dan Nov 24 '15 at 09:55
  • @JamesThorpe: Exactly, the first example might give performance issues. But I have a nasty example when I communicated with a COM-based API, where there were memory leaks in the COM object, so holding the connection open actually were more time consuming than the first example. – Alex Nov 24 '15 at 09:56
  • But the only negative effect of the first example is that some connections might take time to set up and tear down? – Alex Nov 24 '15 at 09:57

1 Answers1

0

I would recommend disposing of any unmanaged resources as soon as possible, as you outline in your example. Garbage collection will get there eventually, but why wait? Comprehensive answer here: Proper use of the IDisposable interface

Specifically re SQL server the connection pool has an upper limit of connections, either a default or defined in your connection string. If you don't close the connections you open, you will exhaust the pool.

Agree with James Thorpe comment that this might depend on the cost of creating the resource, but the examples you specify (db connection, web service call) shouldn't be particularly expensive

Community
  • 1
  • 1
Matt Evans
  • 7,113
  • 7
  • 32
  • 64
  • 1
    Thanks @matthew Evans. Then I will keep on doing this "my way", but always keep in mind the cost of creating the resource. – Alex Nov 24 '15 at 11:47