Non-ORM patterns / practices to abstract database interaction in C#?

Question

I am a database / SQL / ADO.NET newbie, and I'm trying to build a solid foundation before I try to learn about ORM frameworks like NHibernate or Entity Framework. I have been reading up about platform-agnostic RDBMS concepts, and now I'm trying to adapt that to writing clean and reusable C# code that actually interacts with a database. I've heard of Martin Fowler's Patterns of Enterprise Application Architecture which has been recommended here before, but the "Enterprise" in the title makes me wonder if there's not some intermediate learning step I need to make before I'm ready to read that book.

As practice, I'm making a simple ASP.NET web site that helps visitors look up certain information from a database that has a few thousand records. It's a painfully simple first project--there are no user accounts or logins of any kind, and the site would get about a dozen hits per day.

My ASP.NET code features a few methods like this to retrieve data from the database:

string dbConnectString = "...";

public List<string> GetItems()
   {
   List<string> rList = null;

   using (SqlConnection myConn = new SqlConnection(dbConnectString))
      {
      string queryStatement = "SELECT Name FROM SomeTable";

      using (SqlCommand myCmd = new SqlCommand(queryStatement, myConn))
         {
         DataTable resultTable = new DataTable("Results");
         using (SqlDataAdapter myAdapter = new SqlDataAdapter(myCmd))
            {
            myConn.Open();
            if (myAdapter.Fill(resultTable) > 0)
               {
               // This loop should probably be a LINQ expression
               rList = new List<string>();
               foreach (DataRow row in resultTable.Rows)
                  {
                  rList.Add((string)row["Name"]);
                  }
               }

            myConn.Close();
            }
         }
      }

   return rList;
   }

I think I've done the right thing as far as handling the objects that implement IDisposable. But is there a better way to write the method itself?

Here are some questions that I have:

1. From what I've read, it seems that connection pooling makes it OK to instantiate a new SqlConnection with each call to methods like GetItems(). Is there a "cleaner" way to write the code, so I still do proper resource management, but don't have so much repetition in blocks like using (SqlConnection ...)? How about this approach:

   public class DatabaseManager : IDisposable
      {
      protected SqlConnection myConn;
   
      public DatabaseManager(string connectionString)
         {
         // Set up the constructor
         myConn = new SqlConnection(dbConnectString);
         }
   
      // IDisposable implementation stuff goes here.  Any usage of 
      // DatabaseManager would have to take place in a using() block.
   
      public List<string> GetItems()
         {
         List<string> rList = null;
   
         string queryStatement = "SELECT Name FROM dbo.SomeTable";
   
         using (SqlCommand myCmd = new SqlCommand(queryStatement, myConn))
            {
            DataTable resultTable = new DataTable("Results");
            using (SqlDataAdapter myAdapter = new SqlDataAdapter(myCmd))
               {
               myConn.Open();
               if (myAdapter.Fill(resultTable) > 0)
                  {
                  rList = new List<string>();
                  foreach (DataRow row in resultTable.Rows)
                     {
                     rList.Add((string)row["Name"]);
                     }
                  }
   
               myConn.Close();
               }
            }
   
         return rList;
         }
      }
   

2. Aside from my shameless use of vertical spacing, is there a more elegant way to extract the List from the result of the database query? One way would be to replace the foreach() iteration with a LINQ call, but that would only save two or three lines of code.

Thanks guys!

Answer 1

Your code is neither clean nor reusable:

1. The sql query and column name are hardcoded into the manager class which makes is necessary to have an extra manager for each single table. You could possibly solve this by allowing actual queries to be injected but then you'll end up with a "manager of managers" with different responsibilities
2. The lifetime of the sql connection is tied to the lifetime of the manager which makes it impossible to have a transaction over a compound database access
3. There is no error handling
4. Using a data adapter to first retrieve the data and then copy it to a list is less effective than using a data reader

To fix all these issues you need a lot of time and a lot of clever ideas. You have issues with a single manager with a single select query. How about dozens of managers, selects, updates, inserts, deletes with joins to other managers and all this with proper transaction scopes and error handling?

This is why people stick with ORMs when possible and do not reinvent wheels. Writing a good, reusable data access layer is not easy and although you could possibly have a working solution for one or two simple cases, this would not scale well because of reasons I've mentioned. Sooner or later you'll end up with a large pile of pure mess and after two or three unsuccesfull aproaches you would end up reinventing ORMs.