Thanks for this excellent responses. I agree with Ben that code consistency
is important and I am trying to enforce it in my team. Well, the reason I
posted this code is to see the dev reaction on this topic. Pattern beginners
think that pattern is the panacea to all oops problems. This singleton code
was written with great passion [I guess, little to much] and the idea was
great - to optimize the resource utilization. But the dev who wrote this
code was new to the patterns and could not foresee the threading / pooling
problems.


So this discussion was really educational for me.

And one more thing, I personally think that grading the code as A or D is
not a good idea. We are not evaluating a person / dev but the code. What do
you think? Well - I revised my code review comments to - Please check the
discussion on this thread and do the needful.



Thanks.

Abhijit


On 10/17/07, Baris Acar <[EMAIL PROTECTED]> wrote:
>
> why not use Enterprise Data Application Block?
>
> On 10/17/07, Ben Joyce <[EMAIL PROTECTED]> wrote:
> > Many thanks for the clarification, gentelmen.
> >
> > On 10/17/07, Richard Blewett <[EMAIL PROTECTED]> wrote:
> > > Dispose also returns the connection to the connection pool
> > >
> > > Regards
> > >
> > > Richard Blewett - DevelopMentor
> > >
> > > > -----Original Message-----
> > > > From: Discussion of advanced .NET topics. [mailto:ADVANCED-
> > > > [EMAIL PROTECTED] On Behalf Of Smotritsky, Alex
> > > > Sent: 16 October 2007 22:01
> > > > To: [email protected]
> > > > Subject: Re: [ADVANCED-DOTNET] Singleton and Database Connection
> > > > challange
> > > >
> > > > The open and close methods of the connection object in ado.netusually
> > > > take the connection from the connection pool on open and return it
> on
> > > > close so you get connection pooling for free.
> > > >
> > > > -----Original Message-----
> > > > From: Discussion of advanced .NET topics.
> > > > [mailto:[EMAIL PROTECTED] On Behalf Of Mike
> Andrews
> > > > Sent: Tuesday, October 16, 2007 2:34 PM
> > > > To: [email protected]
> > > > Subject: Re: [ADVANCED-DOTNET] Singleton and Database Connection
> > > > challange
> > > >
> > > > This is not the place for a singleton and the singleton is not
> > > > implemented
> > > > correctly since it doesn't take into account any race conditions
> that
> > > > might
> > > > occur with a multi-threaded app such as asp.net.  Also you would not
> > > > want to
> > > > dispose of the connection in the singleton since that defeats the
> > > > purpose of
> > > > the singleton.
> > > > The connection should be created and destroyed when needed and not
> kept
> > > > live
> > > > by using the singleton.
> > > >
> > > >
> > > > On 10/16/07, Abhijit Gadkari <[EMAIL PROTECTED]> wrote:
> > > > >
> > > > > We are writing an ASP.NET application. We have used singleton for
> > > > > instantiating a database connection as explained in the following
> > > > code
> > > > > sample.
> > > > >
> > > > >
> > > > >
> > > > > Design thought was to use only one instance of MyDb across all the
> > > > pages
> > > > > in
> > > > > this web application. Is this a correct singleton implementation
> for
> > > > > Asp.NETweb application for managing database connection? In fact,
> > > > this
> > > > > code got C -
> > > > > or D in our internal code review. Don't know why? Any idea on how
> to
> > > > > improve
> > > > > this code to A level.
> > > > >
> > > > >
> > > > >
> > > > > public sealed class AppDbProvider : IDisposable
> > > > >
> > > > >    {
> > > > >
> > > > >        public static readonly AppDbProvider instance = new
> > > > > AppDbProvider();
> > > > >
> > > > >
> > > > >
> > > > >        private readonly MyDb _d = new MyDb(ConfigurationManager
> > > > > .ConnectionStrings["my_connection"].ConnectionString,
> > > > >
> > > > >
> ConfigurationManager.ConnectionStrings[
> > > > > "my_connection"].ProviderName);
> > > > >
> > > > >
> > > > >
> > > > >        public MyDb GetDbConnection()
> > > > >
> > > > >        {
> > > > >
> > > > >            if (_d != null)
> > > > >
> > > > >                return _d;
> > > > >
> > > > >            else
> > > > >
> > > > >                throw new Exception("Problem with Database
> Connection
> > > > in
> > > > > AppDbProvider.");
> > > > >
> > > > >        }
> > > > >
> > > > >
> > > > >
> > > > >        #region IDisposable Members
> > > > >
> > > > >
> > > > >
> > > > >        public void Dispose()
> > > > >
> > > > >        {
> > > > >
> > > > >            if (_d != null)
> > > > >
> > > > >                _d.Dispose();
> > > > >
> > > > >
> > > > >
> > > > >                Dispose();
> > > > >
> > > > >
> > > > >
> > > > >        }
> > > > >
> > > > >
> > > > >
> > > > >        #endregion
> > > > >
> > > > >    }
> > > > >
> > > > >
> > > > >
> > > > > Now in code behind file, we have following code
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > public partial class CreatePage
> > > > >
> > > > >    {
> > > > >
> > > > > private AmgDb _d;
> > > > >
> > > > >
> > > > >
> > > > > try
> > > > >
> > > > > {
> > > > >
> > > > >     _d = AppDbProvider.instance.GetDbConnection();
> > > > >
> > > > > }
> > > > >
> > > > > Catch (Exception exce)
> > > > >
> > > > > {
> > > > >
> > > > > Response.write(exce.message);
> > > > >
> > > > > }
> > > > >
> > > > >
> > > > >
> > > > >     }
> > > > >
> > > > >
> > > > >
> > > > > And in the end, in the page unload event we have following cleanup
> > > > code
> > > > >
> > > > >
> > > > >
> > > > > protected void Page_Unload(object sender, EventArgs e)
> > > > >
> > > > >        {
> > > > >
> > > > >            if (_d != null)
> > > > >
> > > > >            {
> > > > >
> > > > >                _d = null;
> > > > >
> > > > >            }
> > > > >
> > > > >        }
> > > > >
> > > > > ===================================
> > > > > This list is hosted by DevelopMentor(r)  http://www.develop.com
> > > > >
> > > > > View archives and manage your subscription(s) at
> > > > > http://discuss.develop.com
> > > > >
> > > >
> > > > ===================================
> > > > This list is hosted by DevelopMentor(r)  http://www.develop.com
> > > >
> > > > View archives and manage your subscription(s) at
> > > > http://discuss.develop.com
> > > >
> > > > ===================================
> > > > This list is hosted by DevelopMentor.  http://www.develop.com
> > > >
> > > > View archives and manage your subscription(s) at
> > > > http://discuss.develop.com
> > >
> > > ===================================
> > > This list is hosted by DevelopMentor(r)  http://www.develop.com
> > >
> > > View archives and manage your subscription(s) at
> http://discuss.develop.com
> > >
> >
> >
> > --
> > http://www.flickr.com/photos/benbenbenbenben
> >
> > ===================================
> > This list is hosted by DevelopMentor(r)  http://www.develop.com
> >
> > View archives and manage your subscription(s) at
> http://discuss.develop.com
> >
>
>
> --
> Devrim Baris Acar
> http://www.barisacar.com
>
> ===================================
> This list is hosted by DevelopMentor(r)  http://www.develop.com
>
> View archives and manage your subscription(s) at
> http://discuss.develop.com
>

===================================
This list is hosted by DevelopMentor®  http://www.develop.com

View archives and manage your subscription(s) at http://discuss.develop.com

Reply via email to