Types that own disposable fields must be disposable. How to solve this warning?

Question

I tried to use the Run-time code analysis option in VisualStudio 2012 , as a result of which I received a warning like

 CA1001 Types that own disposable fields should be disposable Implement IDisposable on 'DBConnectivity' because it creates members of the following IDisposable types: 'SqlConnection', 'SqlCommand'. 

I called some question in SO, but I could not understand what is IDisposable and the next is the class responsible for this warning.

 class DBConnectivity { public SqlConnection connection = null; public SqlCommand command = null; public SqlDataReader dataReader = null; public string connectionString = null; public List<MasterTableAttributes> masterTableList; public DBConnectivity() { connectionString = ConfigurationManager.ConnectionStrings["Master"].ConnectionString; connection = new SqlConnection(connectionString.ToString()); //-----Master table results connection.Open(); string masterSelectQuery = "SELECT * FROM MASTER_TABLE"; command = new SqlCommand(masterSelectQuery, connection); dataReader = command.ExecuteReader(); masterTableList = new List<MasterTableAttributes>(); while (dataReader.Read()) { MasterTableAttributes masterTableAttribute = new MasterTableAttributes() { fileId = Convert.ToInt32(dataReader["Id"]), fileName = Convert.ToString(dataReader["FileName"]), frequency = Convert.ToString(dataReader["Frequency"]), scheduledTime = Convert.ToString(dataReader["Scheduled_Time"]) }; masterTableList.Add(masterTableAttribute); } dataReader.Close(); connection.Close(); } } 

I am really confused about the implementation of IDisposable. Any help appreciated?

Answer 1

I completely agree with the compiler - you need to manage your fields here or (as already noted) - do not create fields in the first place:

 class DBConnectivity : IDisposable // caveat! read below first { public void Dispose() { if(connection != null) { connection.Dispose(); connection = null; } if(command != null) { command.Dispose(); command = null; } if(dataReader != null) { dataReader.Dispose(); dataReader = null; } } 

Note that you would use this type with using(...)

---

But! It seems like a static method would be more appropriate:

 static class DBConnectivity { public static List<MasterTableAttributes> GetMasterTableList() { var connectionString = ConfigurationManager.ConnectionStrings["Master"].ConnectionString; using(var connection = new SqlConnection(connectionString)) { connection.Open(); const string masterSelectQuery = "SELECT * FROM MASTER_TABLE"; using(var command = new SqlCommand(masterSelectQuery, connection)) using(var dataReader = command.ExecuteReader()) { var masterTableList = new List<MasterTableAttributes>(); while (dataReader.Read()) { MasterTableAttributes masterTableAttribute = new MasterTableAttributes() { fileId = Convert.ToInt32(dataReader["Id"]), fileName = Convert.ToString(dataReader["FileName"]), frequency = Convert.ToString(dataReader["Frequency"]), scheduledTime = Convert.ToString(dataReader["Scheduled_Time"]) }; masterTableList.Add(masterTableAttribute); } return masterTableList; } } } } 

or perhaps simpler with a tool like dapper:

 static class DBConnectivity { public static List<MasterTableAttributes> GetMasterTableList() { var connectionString = ConfigurationManager.ConnectionStrings["Master"].ConnectionString; using(var connection = new SqlConnection(connectionString)) { connection.Open(); const string sql = "SELECT Id as [FileId], FileName, Frequency, Scheduled_Time as [ScheduledTime] FROM MASTER_TABLE"; return connection.Query<MasterTableAttributes>(sql).ToList(); } } } 

Answer 2

If this is a complete class, you must move all the SQL variables inside the constructor. Or maybe change the constructor to a static function that returns masterTableList