Answered by:
Disposing Objects in WebAPI

Question
-
User-574293449 posted
I am going to maintain one existing API implementation. When i look into the code i got that there are some issue with object dispose.
The following are my Base Controller which acts as parent controller for all.
Base Controller
[LoggingFilter] [System.Web.Http.Authorize] public abstract class BaseV1Controller : ApiController { private ModelFactoryV1 _modelFactoryV1; private MyDBContext __db; private MyLoggingService _loggingService; private int _customerId; protected string __IPAddress; protected ILogger __logger; protected const int PAGE_SIZE_NORMAL = 20; protected const int PAGE_SIZE_MEDIA = 2; // GET: Base protected string __loggingResourceName = "Undefined - base controller"; private void InitLogger() { Log.Logger.ForContext<BaseV1Controller>(); } protected MyDBContext _db { get { return __db; } set { __db = value; } } public BaseV1Controller() { IEnumerable<string> values; __db = new MyDBContext(); _loggingService = new MyLoggingService (); InitLogger(); } public BaseV1Controller(MyDBContext db) { __db = db; _loggingService = new MyLoggingService (); InitLogger(); } protected override void Dispose(bool disposing) { base.Dispose(disposing); _loggingService = null; } }
We are not overriding dispose method in the controller. In the controller we are calling Repository classes to do CRUD operations .
Sample implementation below;
Controller:
[LoggingFilter] [ValidateModel] [Authorize] public class CustomersV1Controller : BaseV1Controller { IAsyncRepository<Customer> _repo = new CustomerAsyncRepository(); public CustomersV1Controller() : base() { _repo = new CustomerAsyncRepository(); } public CustomersV1Controller(IAsyncRepository<Customer> repo, MyDBContext db) : base(db) { __loggingResourceName = "Customer"; _repo = repo; } //All Actions implemented here }
Repository Interface and Class:
public interface IAsyncRepository<T> { Task<T> Add(T type); Task<T> Get(int Id); Task Update(T type); }
public class CustomerAsyncRepository : IAsyncRepository<Customer> { //saves the customer view models private MyDBContext _db { get; } public CustomerAsyncRepository(MyDBContext db) { this._db = db; } public CustomerAsyncRepository() { _db = new MyDBContext (); } public async Task<Customer> Add(Customer model) { //Add method implmementation return model; } public async Task<Customer> Get(int id) { //Implementation to return customer model } public async Task Update(Customer model) { //Implementation to update customer model } }
I have the following clarification based on this
- I think we should include _db.Dispose() in the dispose method of BaseV1Controller. Please suggest?
- In Repository IDisposable not implemented. Is this correct?
- Any other improvements?
Monday, September 17, 2018 9:54 AM
Answers
-
User1120430333 posted
I have the following clarification based on this
I think we should include _db.Dispose() in the dispose method of BaseV1Controller. Please suggest?
In Repository IDisposable not implemented. Is this correct?
Any other improvements?Why not just use an IoC and let it control the instancing and disposing of the objects?
I am not a fan of a generic repository. I don't consider the repository object as a persistence object but rather it's a domain object.
https://programmingwithmosh.com/entity-framework/common-mistakes-with-the-repository-pattern/
https://martinfowler.com/eaaCatalog/repository.html
<copied>
A Repository mediates between the domain and data mapping layers, acting like an in-memory domain object collection. Client objects construct query specifications declaratively and submit them to Repository for satisfaction. Objects can be added to and removed from the Repository, as they can from a simple collection of objects, and the mapping code encapsulated by the Repository will carry out the appropriate operations behind the scenes. Conceptually, a Repository encapsulates the set of objects persisted in a data store and the operations performed over them, providing a more object-oriented view of the persistence layer. Repository also supports the objective of achieving a clean separation and one-way dependency between the domain and data mapping layers.
<end>
https://martinfowler.com/eaaCatalog/dataMapper.html
On the other hand, I don't mess with the Repository pattern if the Domain is anemic with the Repository being no more than a glorified DAO, and I'll just use the DAL with the DAO pattern and eliminate the Repository.
http://blog.sapiensworks.com/post/2012/11/01/Repository-vs-DAO.aspx
I am using the Data Access Object pattern, but usage of the IoC like Unity would be kind of the same for repository or DAO pattern.
https://www.tutorialspoint.com/design_pattern/data_access_object_pattern.htm
I am not telling you not to use a generic repositroy. It's just a little FYI.
Imports System.Data.Entity Imports System.Web.Http Imports Unity Imports DAL Imports Unity.Lifetime Public Module WebApiConfig Public Sub Register(ByVal config As HttpConfiguration) ' Web API configuration and services dim container = new UnityContainer() container.RegisterType(Of IDaoProject, DaoProject) () container.RegisterType(Of IDaoTask, DaoTask) () container.RegisterType(Of IDaoCache, DaoCache) () container.RegisterType(Of DbContext, ProjectManagementEntities)(new HierarchicalLifetimeManager()) config.DependencyResolver = new UnityResolver(container) ' Web API routes config.MapHttpAttributeRoutes() config.Routes.MapHttpRoute( name:="DefaultApi", routeTemplate:="api/{controller}/{action}/{id}", defaults:=New With {.controller = "Home", .action = "Index", .id = UrlParameter.Optional} ) End Sub End Module
Imports System.Web.Http Imports DAL Imports Entities Namespace Controllers <CustomExceptionFilter> Public Class ProjectController Inherits ApiController Private ReadOnly _daoproject As IDaoProject public sub New (daoproject As IDaoProject) _daoproject = daoproject End sub <HttpGet> <ActionName("GetProjectById")> public Function GetProjectById(ByVal id As Int32) As DtoProject return _daoproject.GetProjectById(id) End Function <HttpGet> <ActionName("GetProjectsByUserId")> public Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject) return _daoproject.GetProjectsByUserId(userid) End Function <HttpPost> <ActionName("CreateProject")> public sub CreateProject(ByVal dto As DtoProject) Call _daoproject.CreateProject(dto) End sub <HttpPost> <ActionName("UpdateProject")> public sub UpdateProject(ByVal dto As DtoProject) Call _daoproject.UpdateProject(dto) End sub <HttpPost> <ActionName("DeleteProject")> public sub DeleteProject(ByVal dto As DtoId) Call _daoproject.DeleteProject(dto.Id) End sub End Class End Name
Imports Entities Public Interface IDaoProject Function GetProjectById(ByVal id As Int32) As DtoProject Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject) Sub CreateProject(ByVal dto As DtoProject) Sub UpdateProject(ByVal dto As DtoProject) Sub DeleteProject(ByVal id As Int32) End Interface ==================================================================== Imports System.Data.Entity Imports Entities Public Class DaoProject Implements IDaoProject Private ReadOnly context As ProjectManagementEntities public sub New (dbcontext As ProjectManagementEntities) context = dbcontext End sub Public Function GetProjectById(ByVal id As Int32) As DtoProject Implements IDaoProject.GetProjectById Dim dto = New DtoProject() Dim project = (context.Projects.Where(Function(a) a.ProjectId = id)).SingleOrDefault() If IsNothing(project) Then Return dto End If dto.ProjectId = project.ProjectId dto.ClientName = project.ClientName dto.ProjectName = project.ProjectName dto.Technology = project.Technology dto.ProjectType = project.ProjectType dto.UserId = project.UserId dto.StartDate = project.StartDate dto.EndDate = project.EndDate dto.Cost = project.Cost Return dto End Function Public Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject) Implements IDaoProject.GetProjectsByUserId Dim dtos = New List(Of DtoProject) dtos = (From a In context.Projects.Where(Function(a) a.UserId.Contains(userid)) Select New DtoProject With {.ProjectId = a.ProjectId, .ClientName = a.ClientName, .ProjectName = a.ProjectName, .Technology = a.Technology, .ProjectType = a.ProjectType, .UserId = a.UserId, .StartDate = a.StartDate, .EndDate = a.EndDate, .Cost = a.Cost}).ToList() Return dtos End Function Public Sub CreateProject(ByVal dto As DtoProject) Implements IDaoProject.CreateProject Dim project = New Project() With {.ClientName = dto.ClientName, .ProjectName = dto.ProjectName, .Technology = dto.Technology, .ProjectType = dto.ProjectType, .UserId = dto.UserId, .StartDate = dto.StartDate, .EndDate = dto.EndDate, .Cost = dto.Cost} context.Projects.Add(project) context.SaveChanges() End Sub Public Sub UpdateProject(ByVal dto As DtoProject) Implements IDaoProject.UpdateProject Dim project = New Project() project = (context.Projects.Where(Function(a) a.ProjectId = dto.ProjectId)).SingleOrDefault() If Not IsNothing(project) Then project.ClientName = dto.ClientName project.ProjectName = dto.ProjectName project.Technology = dto.Technology project.ProjectType = dto.ProjectType project.UserId = dto.UserId project.StartDate = dto.StartDate project.EndDate = dto.EndDate project.Cost = dto.Cost End If If IsNothing(project) Then Exit Sub End If context.Entry(project).State = EntityState.Modified context.SaveChanges() End Sub Public Sub DeleteProject(ByVal id As Int32) Implements IDaoProject.DeleteProject Dim project As Project project = (context.Projects.Where(Function(a) a.ProjectId = id)).Include("Tasks").SingleOrDefault() If IsNothing(project) Then Exit Sub End If For i As Integer = 0 To project.Tasks.Count - 1 Dim task = project.Tasks(i) context.Entry(task).State = EntityState.Deleted Next context.Entry(project).State = EntityState.Deleted context.SaveChanges() End Sub
End Class- Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
Monday, September 17, 2018 11:01 AM -
User-474980206 posted
typically you implement Dispose, when your class create a Disposable object without a using statement. Controller implement Dispose, because this is a common pattern. if you create a db context in your constructor to be used by actions, then you need to dispose it in the dispose:
pubic class MyController : Controller { private MyContext context = null public MyController() { context = new MyContext(); // must be disposed } ...... protected override void Dispose(bool disposing) { base.Dispose(disposing); if (disposing && context != null) context.Dispose(); } }
now you code is buggy, because if your constructor allocates the context, then its not disposed. also if you _loggingService needs Disposing, then setting to null, does not do it. actually there is no need to set variables to null in the disposer, because the runtime will do this in any event. you should only be calling Dispose() in the method, or release unmanaged resources you allocated.
- Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
Wednesday, September 26, 2018 3:48 PM
All replies
-
User1120430333 posted
I have the following clarification based on this
I think we should include _db.Dispose() in the dispose method of BaseV1Controller. Please suggest?
In Repository IDisposable not implemented. Is this correct?
Any other improvements?Why not just use an IoC and let it control the instancing and disposing of the objects?
I am not a fan of a generic repository. I don't consider the repository object as a persistence object but rather it's a domain object.
https://programmingwithmosh.com/entity-framework/common-mistakes-with-the-repository-pattern/
https://martinfowler.com/eaaCatalog/repository.html
<copied>
A Repository mediates between the domain and data mapping layers, acting like an in-memory domain object collection. Client objects construct query specifications declaratively and submit them to Repository for satisfaction. Objects can be added to and removed from the Repository, as they can from a simple collection of objects, and the mapping code encapsulated by the Repository will carry out the appropriate operations behind the scenes. Conceptually, a Repository encapsulates the set of objects persisted in a data store and the operations performed over them, providing a more object-oriented view of the persistence layer. Repository also supports the objective of achieving a clean separation and one-way dependency between the domain and data mapping layers.
<end>
https://martinfowler.com/eaaCatalog/dataMapper.html
On the other hand, I don't mess with the Repository pattern if the Domain is anemic with the Repository being no more than a glorified DAO, and I'll just use the DAL with the DAO pattern and eliminate the Repository.
http://blog.sapiensworks.com/post/2012/11/01/Repository-vs-DAO.aspx
I am using the Data Access Object pattern, but usage of the IoC like Unity would be kind of the same for repository or DAO pattern.
https://www.tutorialspoint.com/design_pattern/data_access_object_pattern.htm
I am not telling you not to use a generic repositroy. It's just a little FYI.
Imports System.Data.Entity Imports System.Web.Http Imports Unity Imports DAL Imports Unity.Lifetime Public Module WebApiConfig Public Sub Register(ByVal config As HttpConfiguration) ' Web API configuration and services dim container = new UnityContainer() container.RegisterType(Of IDaoProject, DaoProject) () container.RegisterType(Of IDaoTask, DaoTask) () container.RegisterType(Of IDaoCache, DaoCache) () container.RegisterType(Of DbContext, ProjectManagementEntities)(new HierarchicalLifetimeManager()) config.DependencyResolver = new UnityResolver(container) ' Web API routes config.MapHttpAttributeRoutes() config.Routes.MapHttpRoute( name:="DefaultApi", routeTemplate:="api/{controller}/{action}/{id}", defaults:=New With {.controller = "Home", .action = "Index", .id = UrlParameter.Optional} ) End Sub End Module
Imports System.Web.Http Imports DAL Imports Entities Namespace Controllers <CustomExceptionFilter> Public Class ProjectController Inherits ApiController Private ReadOnly _daoproject As IDaoProject public sub New (daoproject As IDaoProject) _daoproject = daoproject End sub <HttpGet> <ActionName("GetProjectById")> public Function GetProjectById(ByVal id As Int32) As DtoProject return _daoproject.GetProjectById(id) End Function <HttpGet> <ActionName("GetProjectsByUserId")> public Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject) return _daoproject.GetProjectsByUserId(userid) End Function <HttpPost> <ActionName("CreateProject")> public sub CreateProject(ByVal dto As DtoProject) Call _daoproject.CreateProject(dto) End sub <HttpPost> <ActionName("UpdateProject")> public sub UpdateProject(ByVal dto As DtoProject) Call _daoproject.UpdateProject(dto) End sub <HttpPost> <ActionName("DeleteProject")> public sub DeleteProject(ByVal dto As DtoId) Call _daoproject.DeleteProject(dto.Id) End sub End Class End Name
Imports Entities Public Interface IDaoProject Function GetProjectById(ByVal id As Int32) As DtoProject Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject) Sub CreateProject(ByVal dto As DtoProject) Sub UpdateProject(ByVal dto As DtoProject) Sub DeleteProject(ByVal id As Int32) End Interface ==================================================================== Imports System.Data.Entity Imports Entities Public Class DaoProject Implements IDaoProject Private ReadOnly context As ProjectManagementEntities public sub New (dbcontext As ProjectManagementEntities) context = dbcontext End sub Public Function GetProjectById(ByVal id As Int32) As DtoProject Implements IDaoProject.GetProjectById Dim dto = New DtoProject() Dim project = (context.Projects.Where(Function(a) a.ProjectId = id)).SingleOrDefault() If IsNothing(project) Then Return dto End If dto.ProjectId = project.ProjectId dto.ClientName = project.ClientName dto.ProjectName = project.ProjectName dto.Technology = project.Technology dto.ProjectType = project.ProjectType dto.UserId = project.UserId dto.StartDate = project.StartDate dto.EndDate = project.EndDate dto.Cost = project.Cost Return dto End Function Public Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject) Implements IDaoProject.GetProjectsByUserId Dim dtos = New List(Of DtoProject) dtos = (From a In context.Projects.Where(Function(a) a.UserId.Contains(userid)) Select New DtoProject With {.ProjectId = a.ProjectId, .ClientName = a.ClientName, .ProjectName = a.ProjectName, .Technology = a.Technology, .ProjectType = a.ProjectType, .UserId = a.UserId, .StartDate = a.StartDate, .EndDate = a.EndDate, .Cost = a.Cost}).ToList() Return dtos End Function Public Sub CreateProject(ByVal dto As DtoProject) Implements IDaoProject.CreateProject Dim project = New Project() With {.ClientName = dto.ClientName, .ProjectName = dto.ProjectName, .Technology = dto.Technology, .ProjectType = dto.ProjectType, .UserId = dto.UserId, .StartDate = dto.StartDate, .EndDate = dto.EndDate, .Cost = dto.Cost} context.Projects.Add(project) context.SaveChanges() End Sub Public Sub UpdateProject(ByVal dto As DtoProject) Implements IDaoProject.UpdateProject Dim project = New Project() project = (context.Projects.Where(Function(a) a.ProjectId = dto.ProjectId)).SingleOrDefault() If Not IsNothing(project) Then project.ClientName = dto.ClientName project.ProjectName = dto.ProjectName project.Technology = dto.Technology project.ProjectType = dto.ProjectType project.UserId = dto.UserId project.StartDate = dto.StartDate project.EndDate = dto.EndDate project.Cost = dto.Cost End If If IsNothing(project) Then Exit Sub End If context.Entry(project).State = EntityState.Modified context.SaveChanges() End Sub Public Sub DeleteProject(ByVal id As Int32) Implements IDaoProject.DeleteProject Dim project As Project project = (context.Projects.Where(Function(a) a.ProjectId = id)).Include("Tasks").SingleOrDefault() If IsNothing(project) Then Exit Sub End If For i As Integer = 0 To project.Tasks.Count - 1 Dim task = project.Tasks(i) context.Entry(task).State = EntityState.Deleted Next context.Entry(project).State = EntityState.Deleted context.SaveChanges() End Sub
End Class- Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
Monday, September 17, 2018 11:01 AM -
User-574293449 posted
<script type="text/javascript" src="//s3.amazonaws.com/js-static/18fe990e484820dae3.js"></script>
I can understand that, if we implement Dependency Injection disposing is not our responsibility. IOC container will do that for us.
Ok. Based on code i understand that the developer who developed this, followed some blog and tries to implement DI. So he makes this Basev1Controller etc. But i Don't know why he din't implemented DI. So he also skipped disposing of object.
So that my confusion is then why he override Dispose method in BaseV1Controller? May be add this after skipped DI pattern. Also we need to include _db.Dispose() also in this method. correct?
Thanks for the explanation and guidance for Repository/ I will look into DAO pattern. Existing implementation looks like this so i don't want to chnage all. I will improve one by one.
Currently i cannot extend this repository classes due to the generic interface. So my plan is to do the following
- Create separate interface from generic interface for each repository
- Create generic repository class for generic methods and extend to thew custom repositories
This will help me to extend the repository related methods. Hence avoid linq and other data access codes from Controller.
Based on this aim, we need to implement IDisposable in the interface? Also include dispose in repository as we are not implement IOC now?
Tuesday, September 18, 2018 1:49 AM -
User1120430333 posted
Based on this aim, we need to implement IDisposable in the interface? Also include dispose in repository as we are not implement IOC now?
I couldn't tell you, becuase the last time I used a Dispose() was around year 2008.
Tuesday, September 18, 2018 2:58 PM -
User-474980206 posted
typically you implement Dispose, when your class create a Disposable object without a using statement. Controller implement Dispose, because this is a common pattern. if you create a db context in your constructor to be used by actions, then you need to dispose it in the dispose:
pubic class MyController : Controller { private MyContext context = null public MyController() { context = new MyContext(); // must be disposed } ...... protected override void Dispose(bool disposing) { base.Dispose(disposing); if (disposing && context != null) context.Dispose(); } }
now you code is buggy, because if your constructor allocates the context, then its not disposed. also if you _loggingService needs Disposing, then setting to null, does not do it. actually there is no need to set variables to null in the disposer, because the runtime will do this in any event. you should only be calling Dispose() in the method, or release unmanaged resources you allocated.
- Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
Wednesday, September 26, 2018 3:48 PM