I'm a little bit familiar with Entity Framework for some simple projects, but now I want to go deeper and write better code.
There is plenty of topics talking about whether using statics methods in DAL or not. For the moment I'm more of the side of people who think yes we can use statics methods.
But I'm still thinking if some practices are good or not.
Lot of people do like this:
public IList<Person> GetAll()
{
using (var dbContext = new MyDbContext())
{
return dbContext.Persons.ToList();
}
}
But I'm wondering if doing like this is a good practice:
public static IQueryable<Person> GetAll()
{
var dbContext = new MyDbContext();
return dbContext.Persons;
}
The goal is to use only static methods in a static class as I think it's legit because this class is just a DAL, there will never be any property. I also need to do like this instead of using the using() scope to avoid disposing the context as this method return an IQueryable.
I'm sure some people already think "OMG no, your context will never be disposed", so please read this article: http://blog.jongallant.com/2012/10/do-i-have-to-call-dispose-on-dbcontext.html
I tried by myself and yes the context is disposed only when I don't need it anymore. I repeat, the goal here is to use static methods so I can't use a dbContext property which the constructor instantiate.
So why people always use the using() scope?
Is it a bad practice to do it like I would like to?
Another bonus question: where is the [NotMapped] attribute with EF6? I've checked on both System.ComponentModel.DataAnnotations and System.ComponentModel.DataAnnotations.Schema but can't find it, this attribute is not recognized by the compiler.
Thank's for your answers
Following the Repository pattern, IQueryable<T> shall never be returned anyway.
Repository pattern, done right
Besides, your repositories depend on your DbContext. Let's say you have to work on customers in an accounting system.
Customer
public class Customer {
public int Id { get; protected set; }
public string GivenName { get; set; }
public string Surname { get; set; }
public string Address { get; set; }
}
CustomerRepository
public class CustomerRepository {
public CustomerRepository(DbContext context) {
if (context == null) throw new ArgumentNullException("context");
this.context = context;
}
public IList<Customer> GetAll() { return context.Customers.ToList(); }
public IList<Invoice> GetInvoicesFor(Customer customer) {
return context.Invoices
.Where(invoice => invoice.Customer.Id == customer.Id)
.ToList();
}
private readonly DbContext context;
}
So in fact, to answer your question in a more concise and precise way, I think neither approach is good. I would more preferably use a DbContext per business concern. When you access let's say the Customers Management features, then instantiate a single DbContext that shall be shared across all of your required repositories, then dispose this very DbContext once you exit this set of features. This way, you shall not have to use Using statements, and your contexts should be managed adequately.
Here's another short and simple good reference for the Repository pattern:
In response to comments from the OP
But actually the point is I don't want to follow the repository pattern. People say "what about if your data source change?" I want to answer, what about if it never change? What the point of having a such powerful class but not using it just in case one day the database provider may change
Actually, the Repository Pattern doesn't only serve the purpose of easier data source change, it also encourages better separation of concerns and a more functional approach closer to the business domain as members in the repository shall all revolve around business terminologies.
For sure the repository itself cannot take over control to dispose a data context or whatever the object it uses to access the underlying data source, since it doesn't belong to it, it is only lended to it so that it can fulfill its tasks.
As for your point about will the data source change someday? No one can predict it. It is most likely to never change in most of the systems I have worked on. A database change is more likely to be seen after 10 years of the initial development for moerdnization purposes. That day, however, you'll understand how the Repository Pattern saves you time and headaches in comparison to a tightly coupled code. I work with tightly coupled code from legacy systems, and I do take the advantages for benefits. Prevention is better than cure.
But please lets focus on instantiate the dbContext in methods without the using() statement. Is it really bad? I mean also when we inject the context in the constructor we don't handle the dispose(), we let entity framework doing it and it manages it pretty well.
No, it isn't necessarily bad not to use using statements, as long as you dispose all unnecessary resources as long as they are no longer used. The using statements serves this purpose for you by doing it automatically instead of you having to take care about.
As for the Repository pattern, it can't dispose the context that is passed to it, and it shan't be disposed neither because the context is actually contextualized to a certain matter and is used across other features within a given business context.
Let's say you have Customer management features. Within them, you might also require to have the invoices for this customer, along with the transaction history. A single data context shall be used across all of the data access as long as the user works within the customer management business context. You shall then have one DbContext injected in your Customer management feature. This very same DbContext shall be shared across all of the repositories used to access your data source.
Once the user exits the Customer management functionalities, the DbContext shall get disposed accordingly as it may cause memory leaks. It is false to believe that as long as it is no longer used, everything gets garbage collected. You never know how the .NET Framework manages its resources and how long it shall take to dispose your DbContext. You only know that it might get disposed somehow, someday.
If the DbContext gets disposed immediately after a data access is performed, you'll have to instantiate a new instance everytime you need to access the underlying data source. It's a matter of common sense. You have to define the context under which the DbContext shall be used, and make it shared across the identified resources, and dispose it as soon as it is no longer needed. Otherwise, it could cause memory leaks and other such problems.
In response to comment by Mick
I would go further and suggest you should always return IQueryable to enable you to reuse that result passing it into other calls on your repositories. Sorry but your argument makes absolutely no sense to me. Repositories are not meant to be stand-alone one stop shops they should be used to break up logic into small, understandable, encapsulated, easily maintained chunks.
I shall disagree to always return IQueryable<T> through a Repository, otherwise what is it good to have multiple methods for? To retrieve the data within your repository, one could simply do:
public class Repository<T> where T : class {
public Repository(DbContext dataContext) { context = dataContext; }
public IQueryable<T> GetAll() { return context.Set<T>(); }
private readonly DbContext context;
}
and place predicates everywhere in your code to filter the data as per the views needs or whatsoever. When it is time to change the filter criterion or the like, you'll have to browse all of your code to make sure anyone didn't use a filter which was actually unexpected, and may cause the system to misbehave.
On a side note, I do understand your point and I might admit that for some reasons as described in your comment, it might be useful to return IQueryable<T>. Besides, I simply wonder for what good is it, since a repository's responsibility is to provide a class everything it needs to get its information data. So if one needs to pass along IQueryable<T>s to another repository, it sounds strange to me as if one wouldn't have completely investigated every possible way to retrieve data. If one need some data to process another query, lazy-loading can do it and no need to return IQueryables. As per its name, IQueryables are made to perform queries, and the Repository's responsibility to to access the data, that is, to perform queries.