I would give this a C- or D as well ... not because the Singleton is
implemented incorrectly but because it shouldn't be a singleton in the
first place.
1) whats the point of being Disposable? You are a singleton ... if
anyone disposes you you trash your internal state and start returning
a disposed connection
2) This code has huge threading problems if everyone is using it since
it uses a single connection. In order to get around this problem you
have to become serial in your db calls on the same connection.
3) this code defeats the purpose of connection pooling provided by the
framework ... why not just remove it as a whole and use connection
pooling?
Cheers,
Greg
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
>
--
Studying for the Turing test
===================================
This list is hosted by DevelopMentor® http://www.develop.com
View archives and manage your subscription(s) at http://discuss.develop.com
