Web service - getting data from SQL and adding to Object I have created a class called CommandMessages

    public class CommandMessages
    {
        public string CommandType { get; set; }
        public string Command { get; set; }
        public DateTime? SendDateTime { get; set; }

        public CommandMessages()
        {
        }

        public CommandMessages(string commandtype, string command, DateTime? senddatetime)
        {
            CommandType = commandtype;
            Command = command;
            SendDateTime = senddatetime;
        }
    }


I also have a web service called GetData

    [WebMethod]
    public CommandMessages GetData()
    {
        var result = new CommandMessages();

        using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString))
        {
            string query = "exec usp_dequeueTestProject";
            SqlCommand command = new SqlCommand(query, connection);
            connection.Open();
            using (SqlDataReader reader = command.ExecuteReader())
            {
                while (reader.Read())
                 result.CommandType = reader.GetString(0);
                 result.Command = reader.GetString(1);
                 result.SendDateTime = reader.GetDateTime(2);
                 reader.Close();
            }
            connection.Close();
        }

        return result;
    }


Please can someone let me know how efficient my web service is being at getting data from the SQL table?  I don't know if this is the best way to do it or not.
This code here is not good

using (SqlDataReader reader = command.ExecuteReader())
{
    while (reader.Read())
        result.CommandType = reader.GetString(0);
        result.Command = reader.GetString(1);
        result.SendDateTime = reader.GetDateTime(2);
        reader.Close();
}


you don't surround your while statement,  so it will only result in your commandtype being set.

You need to use Braces!

using (SqlDataReader reader = command.ExecuteReader())
{
    while (reader.Read())
    {
        result.CommandType = reader.GetString(0);
        result.Command = reader.GetString(1);
        result.SendDateTime = reader.GetDateTime(2);
    }    
    reader.Close();
}


I assume that the reader.Close(); is supposed to be outside the while statement.  the other thing here is that if your reader returns more than 1 record your result will only hold the last record that is read.



You should do something with that ugly call to your connection string as well, instead of this:

    using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString))
    {
         ...


Do something pretty like this:

var connectionString = ConfigurationsManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString;

using (SqlConnection connection = new SqlConnection(connectionString)
{
     ....


This way when you want to change from the Test connection string to the Production String, you only have to change it here and not every place it is mentioned in the code.  it is easier to maintain this way.



I did some research and I you should also use a using statement for you sqlcommand as well (do we need using for the SqlCommand or is it enough just for the SqlConnection and SqlDataReader).

so this:

    using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString))
    {
        string query = "exec usp_dequeueTestProject";
        SqlCommand command = new SqlCommand(query, connection);
        connection.Open();
        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
             result.CommandType = reader.GetString(0);
             result.Command = reader.GetString(1);
             result.SendDateTime = reader.GetDateTime(2);
             reader.Close();
        }
        connection.Close();
    }


Will become this (with all the changes so far):

    using (SqlConnection connection = new SqlConnection(connectionString)
    {
        string query = "exec usp_dequeueTestProject";
        connection.Open();

        using (SqlCommand command = new SqlCommand(query, connection))
        {
            using (SqlDataReader reader = command.ExecuteReader())
            {
                 while (reader.Read())
                 { 
                      result.CommandType = reader.GetString(0);
                      result.Command = reader.GetString(1);
                      result.SendDateTime = reader.GetDateTime(2);
                 }
                 reader.Close();
            }
        }
        connection.Close();
    }


I moved the connection.open before it is called for the command,  it makes more sense there, at least to me. everything inside the using connection statement should need the connection so Open it first.  That variable for the query string should be created with the other variable, outside the using statement.  

Don't clutter your using statements, it will make things difficult when you go to change something and could cause issues later on with the scope.