HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Read from SQL database table, store in list of objects

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
objectsreadsqlstoredatabaselistfromtable

Problem

IList wsl = new List();
 login1 l = new login1();

 string q = "select Id, DomainUrl, IsApproved, Date from website where UserId = @UserId";

 using (MySqlConnection con = new MySqlConnection(WebConfigurationManager.ConnectionStrings["MySqlConnectionString"].ToString()))
 {
      using (MySqlCommand cmd = new MySqlCommand(q, con))
      {
           cmd.Parameters.Add("@UserId", MySqlDbType.Int32).Value = userId;
           con.Open();

           var reader = cmd.ExecuteReader();
           while (reader.Read())
           {
                var ws = new WebSite();
                ws.Id = reader.GetInt32("Id");
                ws.DomainUrl = reader.GetString("DomainUrl");
                ws.IsApproved = reader.GetBoolean("IsApproved");
                ws.User.Id = reader.GetInt32("UserId");
                ws.Date = reader.GetDateTime("Date");
                wsl.Add(ws);
           }
           reader.Close();
           return wsl;
      }
 }


I want to make this code as elegant as possible.

For this particular project I cannot use NHibernate as I have been instructed.

Solution

This code is very tightly coupled with the specific database engine. That makes it hard to change if you decide to go with different database engine. And it's not possible to unit test it at all.

Instead of using the specific MySql classes for connections, etc., change it to use the base interfaces, and then pass the connections and the queries from outside.

Also, I generally avoid using "return" inside a code block like "using" and if/switch, if it does not make the code too clunky. That way the code path is much easier to follow.

Bellow is an example of this particular method as part of some class, which can be reused in many different situations, incl web pages, services, etc. and with different databases, w/o changing the implementation of this class itself.

private IDatabaseProvider _dbProvider; //initialized in ctor, etc.
private IConnectionStringProvider _connectionStringProvider;
private IWebSiteMapper _mapper; //the db schema may change, or you may need to map from other source

public IList GetWebSitesForUser(int userId)
{
    var wsl = new List();
    var con = _dbProvider.GetNewConnection(); //returns IDbConnection
    var cmd = _dbProvider.GetWebSiteListQueryCommand(con, userId); //IDbCommand, and addParam goes there

    conn.Open(); // no need of try/catch or using, as if this fails, no resources are leaked
    using (var reader = cmd.ExecuteReader(CommandBehavior.CloseConnection)) //close con on reader.Close
    {
         while (reader.Read())
         {
             wsl.Add(_mapper.CreateFromReader(reader));
         }
    }

    return wsl;
}

Code Snippets

private IDatabaseProvider _dbProvider; //initialized in ctor, etc.
private IConnectionStringProvider _connectionStringProvider;
private IWebSiteMapper _mapper; //the db schema may change, or you may need to map from other source

public IList<WebSite> GetWebSitesForUser(int userId)
{
    var wsl = new List<WebSite>();
    var con = _dbProvider.GetNewConnection(); //returns IDbConnection
    var cmd = _dbProvider.GetWebSiteListQueryCommand(con, userId); //IDbCommand, and addParam goes there

    conn.Open(); // no need of try/catch or using, as if this fails, no resources are leaked
    using (var reader = cmd.ExecuteReader(CommandBehavior.CloseConnection)) //close con on reader.Close
    {
         while (reader.Read())
         {
             wsl.Add(_mapper.CreateFromReader(reader));
         }
    }

    return wsl;
}

Context

StackExchange Code Review Q#3592, answer score: 5

Revisions (0)

No revisions yet.