4

1)

public class DataProvider : IProvider , IDisposable{

       private SqlConnection connection = null;
       public DataProvider(string ConnectionString) {
            this.connection = new SqlConnection(ConnectionString);
           this.connection.Open();
       }

       public object GetUniqueData(SqlCommand CommandSql){}

       public void ExecuteInsertDeleteUpdate(SqlCommand CommandSql){}

       public void Dispose(){
          if (this.connection != null) {
              this.connection.Close();
              this.connection.Dispose();
          }
       }
}

2)

public class ManageBrandDAL : IManageBrandDAL {

      private IProvider provider = null;

      [Inject]
      public ManageBrandDAL (IProvider provider_){
           this.provider  = provider_;
      }

      public void RegisterBrand(string a_BrandName){
           SqlCommand SQLCommand =
               new SqlCommand("INSERT INTO Brand(name) VALUES(@pm_brandname)");
           SqlParameter pm_brandname= new SqlParameter();
           pm_brandname.ParameterName = "@pm_brandname";
           pm_brandname.DbType = DbType.String;
           pm_brandname.Value = a_BrandName;
           SQLCommand.Parameters.Add(pm_brandname);
           this.provider.ExecuteInsertDeleteUpdate(SQLCommand);
       }

3)

public class ModuleInfra : Ninject.Modules.NinjectModule
{
      public override void Load(){
            Bind<IProvider>()
                .To<ProvedorDados()
                .InTransientScope()
                .WithConstructorArgument("ConnectionString", Manage.ConnectionString);
      }
}

How can I guarantee that the Ninject Container will call the method Dispose() in DataProvider class after ManageBrandDAL uses the DataProvider object?

Is the InTransientScope() the best lifecycle for this type of situation ? If not, what is more appropriate?

Yuck
  • 49,664
  • 13
  • 105
  • 135
rol
  • 51
  • 1
  • 2
  • 2
    Why do you leave the connection open your `DataProvider` class? If you open and close the connection inside a single method, you don't need to implement `DIsposable`. – Steven Dec 12 '12 at 01:05
  • 1
    See http://stackoverflow.com/questions/10234802/guidelines-for-dispose-and-ninject – Daniel Marbach Dec 12 '12 at 05:16
  • In addition to leaving your database connection open, I believe there is also a general rule that constructors shouldn't throw exceptions. Your design is very prone to that happening. – Yuck Dec 14 '12 at 21:27
  • https://github.com/ninject/ninject/wiki/Object-Scopes then look here http://kohari.org/2009/03/06/cache-and-collect-lifecycle-management-in-ninject-20/ – dotjoe Dec 14 '12 at 21:38
  • @Yuck What's the rationale behind avoiding throwing exceptions in constructors? The only place I've ever seen such a thing is on Symbian OS, where constraints imposed by the platform made ugly hacks like convention-based 2-phase construction a necessity. – anton.burger Dec 14 '12 at 21:41

1 Answers1

4

When you bind your DataProvider InTransientScope() it will not be disposed by Ninject, because actually transient scope is no scope at all. Ninject does not tracks objects that are bound in transient scope after it will create one for you.

Ninject disposes instances of objects implementing IDisposable as soon as the underlying scope object is collected by GC (but as as I said this does not work for objects bound to transient scope because there is no such scope object).

You should bind your DataProvider in scope, that is appropriate to your application. It could be:

  • InRequestScope() for web application (Ninject will dispose instances of objects implementing IDisposable after the end of the http request - don't forget to include OncePerWebRequest module)
  • InSingletonScope() - instance of object will be re-used for all subsequent requests during the whole lifecycle of application - from my point of view it is not an option for objects holding resources like SqlConnection
  • InThreadScope() - instance of object will be re-used within the same thread
  • InScope() - this could be used for creating custom scopes, so according to type of your application you can consider creating your own custom scope suitable to your needs.

There are also interesting extensions for Ninject that provides additional scope definitions: https://github.com/ninject/ninject.extensions.namedscope/wiki

Hints

  • You don't need to call both Close() and Dispose() on your SqlConnection. Dispose() is enough because it calls Close() internally.
  • I don't see the whole code, but do not forget to dispose the SqlCommand too.
  • If you let creating of instances of ManageBrandDAL on Ninject you don't need to use InjectAttribute on its constructor. It will unties you from using specific IOC provider.
mipe34
  • 5,596
  • 3
  • 26
  • 38
  • Dear Mipe34, What SCOPE is more appropriated for a Windows Service, this application will open/close database connections at each 20 seconds to update some tables ? Thanks, beforehand. – rol Dec 18 '12 at 01:42
  • Well, I don't know much about Windows services. But I would go for custom scope using the `InScope()` method or maybe better for `InNamedScope()` or `InParentScope()` from ninject extensions. Read this article http://www.planetgeek.ch/2010/12/08/how-to-use-the-additional-ninject-scopes-of-namedscope/ about how to use additional scopes. – mipe34 Dec 18 '12 at 09:08
  • How and when are objects in `Transient` scope being disposed then? – Ozkan Sep 17 '18 at 10:09
  • When they are garbage collected. See this: https://stackoverflow.com/questions/10234802/guidelines-for-dispose-and-ninject?noredirect=1&lq=1 – mipe34 Sep 17 '18 at 13:46