0
\$\begingroup\$

Fetching database records and displaying as JSON. Asking for revision to ensure if everything's ok, i.e. connections are properly handled and closed in case of error.

public class Department
{
    public Department(int id, String name)
    {
        this.Id = id;
        this.Name = name;
    }

    public int Id { get; set; }

    public String Name { get; set; }
}


public List<Department> FindAllDepartment()
{
    List<Department> rows = new List<Department>();
    using (SqlConnection sqlConnection = new SqlConnection("Data Source=;Initial Catalog=;Persist Security Info=True;User ID=sa;Password=P@ssw0rd;pooling=true"))
    {
        SqlCommand command = new SqlCommand("SELECT * FROM Employees.dbo.Department", sqlConnection);
        try
        {
            sqlConnection.Open();
            using (SqlDataReader sqlDataReader = command.ExecuteReader())
            {
                while (sqlDataReader.Read())
                    rows.Add(new Department(sqlDataReader.GetInt32(0), sqlDataReader.GetString(1)));
            }
            return rows;
        }
        finally
        {
            sqlConnection.Close();
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$
  using (SqlConnection sqlConnection = new SqlConnection("Data Source=;Initial Catalog=;Persist Security Info=True;User ID=sa;Password=P@ssw0rd;pooling=true"))
  {
      ...
      }
      finally
      {
          sqlConnection.Close();
      }
  }

You're correctly using a using statement for the connection, which will both close and dispose the connection when its scope finishes. So no need to explicit call Close().

Almost (if not) every database related object implements IDisposable - including SqlCommand, so you should encapsulate that in a using as well.

All in all, your method should look like something like this:

public List<Department> FindAllDepartment()
{
  using (SqlConnection sqlConnection = new SqlConnection("Data Source=;Initial Catalog=;Persist Security Info=True;User ID=sa;Password=P@ssw0rd;pooling=true"))
  {
    sqlConnection.Open();

    using (SqlCommand command = new SqlCommand("SELECT * FROM Employees.dbo.Department", sqlConnection))
    using (SqlDataReader sqlDataReader = command.ExecuteReader())
    {
      List<Department> rows = new List<Department>();
      while (sqlDataReader.Read())
      {
        rows.Add(new Department(sqlDataReader.GetInt32(0), sqlDataReader.GetString(1)));
      }

      return rows;
    }
  }
}

where the using statements handle the clean up - even if an exception is thrown.

\$\endgroup\$
  • \$\begingroup\$ Thanks, for using (SqlCommand command, are we missing braces? \$\endgroup\$ – AppDeveloper Oct 9 at 16:50
  • 1
    \$\begingroup\$ @AppDeveloper: No, we're allowed to "stack" using statements as shown :-) \$\endgroup\$ – Henrik Hansen Oct 9 at 16:51
  • \$\begingroup\$ cool beans, very articulate. Can you refer me to any of your comprehensive insert, update or delete posts just for the sake of review. \$\endgroup\$ – AppDeveloper Oct 9 at 16:55
  • \$\begingroup\$ @AppDeveloper: I'm not sure if I understand your question? \$\endgroup\$ – Henrik Hansen Oct 9 at 17:17
  • 2
    \$\begingroup\$ Asking for advice on code yet to be written or implemented is off-topic for this site. Even in a sneaky comment. \$\endgroup\$ – dfhwze Oct 9 at 17:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.