-->
These old forums are deprecated now and set to read-only. We are waiting for you on our new forums!
More modern, Discourse-based and with GitHub/Google/Twitter authentication built-in.

All times are UTC - 5 hours [ DST ]



Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 6 posts ] 
Author Message
 Post subject: Configuration Singleton
PostPosted: Wed Jun 07, 2006 4:44 am 
Beginner
Beginner

Joined: Tue May 23, 2006 12:53 am
Posts: 34
Anybody can spot, what I am missing? It hangs when I unit test the code
Code:
/*
* NHibernateConfigurationSingleton.cs 0.1
*
* Copyright 2006 Ian Escarro. All rights reserved.
* PROPRIETARY/CONFIDENTIAL. Use is subject to license terms.
*/

using System;
using NHibernate;
using NHibernate.Cfg;

namespace Agta.ManilaBulletin.Configurations
{
   /// <summary>
   ///
   /// </summary>
   public class NHibernateConfigurationSingleton
   {
      private Configuration cfg = new Configuration();
      private ISessionFactory factory = null;
      
      private static NHibernateConfigurationSingleton instance = new NHibernateConfigurationSingleton();
      public static NHibernateConfigurationSingleton Instance
      {
         get { return instance; }
      }

      /// <summary>
      /// Creates a new instance of NHibernateConfigurationSingleton.
      /// </summary>
      public NHibernateConfigurationSingleton()
      {
         cfg.AddAssembly("webartiData");
         factory = cfg.BuildSessionFactory();
      }

      public ISession OpenSession()
      {
         return factory.OpenSession();
      }
   }
}
with a simple unit test (but can't even test it!)
Code:
/*
* NHibernateConfigurationSingletonTest.cs 0.1
*
* Copyright 2006 Ian Escarro. All rights reserved.
* PROPRIETARY/CONFIDENTIAL. Use is subject to license terms.
*/

using System;
using NUnit.Framework;

namespace Agta.ManilaBulletin.Configurations.Test
{
   /// <summary>
   ///
   /// </summary>
   [TestFixture] public class NHibernateConfigurationSingletonTest
   {
      /// <summary>
      /// Creates a new instance of NHibernateConfigurationSingletonTest.
      /// </summary>
      public NHibernateConfigurationSingletonTest() {}

      [Test] public void Test1()
      {
         NHibernateConfigurationSingleton c = NHibernateConfigurationSingleton.Instance;
         Console.Write(c.ToString());
      }
   }
}
Thanks!


Top
 Profile  
 
 Post subject:
PostPosted: Wed Jun 07, 2006 11:32 am 
Expert
Expert

Joined: Fri May 13, 2005 5:56 pm
Posts: 308
Location: Santa Barbara, California, USA
i am guessing that your Singleton is calling new Cinfiguration() before it has an instance of itself but I can be sure without loading it in the debugger. regardless, i don't think your Singleton is very defensive (safe). doing something like:

Code:
private static NHibernateConfigurationSingleton instance = new NHibernateConfigurationSingleton();


does nothing to prevent the instance from being initialized more than once. the probability is small, but distinct. imho, you should change your Instance property to a method call:


Code:
public static NHibernateConfigurationSingleton GetInstance()
{
   if (instance== null)
   {
      lock (typeof(NHibernateConfigurationSingleton))
      {
         if (instance == null)
         {
               instance = new NHibernateConfigurationSingleton();
         }
      }
   }
   return instance;
}


i would also move the instantiation of the "cfg" variable inside your class constructor. so in the end you have:

Code:
/*
* NHibernateConfigurationSingleton.cs 0.1
*
* Copyright 2006 Ian Escarro. All rights reserved.
* PROPRIETARY/CONFIDENTIAL. Use is subject to license terms.
*/

using System;
using NHibernate;
using NHibernate.Cfg;

namespace Agta.ManilaBulletin.Configurations
{
   /// <summary>
   ///
   /// </summary>
   public class NHibernateConfigurationSingleton
   {
      private Configuration cfg = null;
      private ISessionFactory factory = null;
      private volitile static NHibernateConfigurationSingleton instance;

      public static NHibernateConfigurationSingleton GetInstance()
      {
            if (instance == null)
            {
                  lock (typeof(NHibernateConfigurationSingleton))
                  {
                        if (instance == null)
                        {
                              instance = new NHibernateConfigurationSingleton();
                        }
                  }
            }
            return instance;
      }
     
      /// <summary>
      /// Creates a new instance of NHibernateConfigurationSingleton.
      /// </summary>
      protected NHibernateConfigurationSingleton()
      {
         cfg = new Configuration();
         cfg.AddAssembly("webartiData");
         factory = cfg.BuildSessionFactory();
      }

      public ISession OpenSession()
      {
         return factory.OpenSession();
      }
   }
}


if you do a search on this board, you will find a thread with several other examples.

-devon

EDIT: modified constructor to 'protected.' thanks for the note grennis. i just copied what was there before. d'oh!


Last edited by devonl on Wed Jun 07, 2006 12:26 pm, edited 1 time in total.

Top
 Profile  
 
 Post subject:
PostPosted: Wed Jun 07, 2006 12:22 pm 
Senior
Senior

Joined: Sat Mar 25, 2006 9:16 am
Posts: 150
The constructor should not be public


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jun 09, 2006 8:37 am 
Contributor
Contributor

Joined: Wed May 11, 2005 4:59 pm
Posts: 1766
Location: Prague, Czech Republic
devonl wrote:
doing something like:

Code:
private static NHibernateConfigurationSingleton instance = new NHibernateConfigurationSingleton();


does nothing to prevent the instance from being initialized more than once.


According to Microsoft, this actually is the right way to do singleton and will not cause race conditions. Class constructors are run in thread-safe manner.


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jun 09, 2006 11:19 am 
Expert
Expert

Joined: Fri May 13, 2005 5:56 pm
Posts: 308
Location: Santa Barbara, California, USA
hmmm, interesting. learn something new everyday. sorry nebulom. my only explanation is that i'm am reatively new to the .NET world from Java.

-devon


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jun 09, 2006 6:28 pm 
Expert
Expert

Joined: Fri May 13, 2005 5:56 pm
Posts: 308
Location: Santa Barbara, California, USA
sergey, you are, of course, completely right. i found this article that describes the .NET way to make a Singleton:

http://msdn.microsoft.com/library/defau ... espatt.asp

so in effect, the original class was semantically correct. based on Microsoft's recommended code, my new Singleton would look like:

Code:
sealed class Singleton
{
    private Configuration cfg = null;
    private ISessionFactory factory = null;

    private Singleton()
    {
        cfg = new Configuration();
        cfg.AddAssembly("webartiData");
        factory = cfg.BuildSessionFactory();
    }

    public static readonly Singleton Instance = new Singleton();

    public ISession OpenSession()
    {
        return factory.OpenSession();
    }
}


Top
 Profile  
 
Display posts from previous:  Sort by  
Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 6 posts ] 

All times are UTC - 5 hours [ DST ]


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
© Copyright 2014, Red Hat Inc. All rights reserved. JBoss and Hibernate are registered trademarks and servicemarks of Red Hat, Inc.