2
\$\begingroup\$

I've started to use extension methods to provide data layer functionality in our project. Our project is an MVC website using Code First Entity Framework. So, I've done things like this:

namespace OurProject.Models
{
    public class Project
    {
        public int ProjectId {get; set;}
        public string ProjectName {get; set;}
        public bool IsActive {get; set;}
    }

    public static partial class ProjectExtensions
    {
        public static IQueryable<Project> GetProjects(this IDbContext db)
        {
            return db.Project.Where(p => p.IsActive);
        }
    }
}

Then for ViewModels, I can do this:

namespace OurProject.Models.ViewModels
{
    public class ProjectViewModel
    {
        public int ProjectId {get; set;}
        public string ProjectName {get; set;}
        public string OtherProperty {get; set;}
        public bool IsActive {get; set;}
    }

    public static partial class ProjectExtensions
    {
        public static IQueryable<ProjectViewModel> GetProjectsFormatted(this IDbContext db)
        {
            return db.GetProjects()
                .Select(p => new ProjectViewModel{
                    ProjectId = p.ProjectId,
                    ProjectName = p.ProjectName,
                    OtherProperty = p.ProjectName + " Whatever",
                    IsActive = p.IsActive
                });
        }
    }
}

Is there any reason I shouldn't be doing this?

\$\endgroup\$
  • \$\begingroup\$ Your dbContext may end up being inundated with a whole lot of methods making it hard to use?? \$\endgroup\$ – dreza Mar 30 '16 at 22:58
  • \$\begingroup\$ That's what I'm worried about. I can change it to this IDbSet<Project> project and then call it with dbContext.Project.GetProjectsFormatted(). But I have some extension methods that would have two separate LINQ queries. I would need to call out the DbContext twice, and I don't know if I can do that with the IDbSet<Project> method. \$\endgroup\$ – Mark Kenyon Mar 31 '16 at 13:04
5
\$\begingroup\$

One file vs. many partials

My first comment is about the ProjectExtensions class itself. I would prefer keeping it together. If for any reason you want to make some consistent change in the extensions it's very hard to do that when they're scattered all over the place.

When to use extension methods

This is the main question: should I do this? My opinion: no. At least not to that extent.

When extension methods were unleashed in c# 3.0, the whole developer community (including me) went on a razzle with them and started creating extensions for just about anything. A bit like the days when text processors with more than one font entered the market. Suddenly we saw club bulletins appear that looked like (and violated) this:

1

The trouble is that with extension methods it's too easy to 'add' methods to a very general type while the scope of these methods is more restricted than the scope of the type itself, even within one namespace. For example, I had extension methods on string that I decided to ditch again later, when wisdom finally took over.

This phenomenon also applies to your extension methods. They are defined on IDbContext. Now I don't know how many IDbContext classes you intend to implement, but who says they're all going to have Project?

This means that you'd have to confine the extension methods to contexts knowing about Project. This, in turn, may imply that you may as well replace the extension methods by instance methods in one specific context class.

When create a method at all?

That's another question. No matter if it's an extension method or an instance method. Is the task even worth creating a generally accessible method? I specifically frown upon this GetProjectsFormatted method. To me, this looks like something that should probably be a service method. Remember the single responsibility principle? A context shouldn't have any knowledge about any format. That's a (domain) service's responsibility. It may make the context with all its paraphernalia less applicable (or targeted) to other parts of the application or other assemblies. Or, worse, you may get tempted to enter a range of FomatXyz methods to meet other specifications.

Alternatives

Finally, there are other ways to deal with this 'soft delete' problem, i.e. the problem that records are deactivated rather than physically deleted, while the deactivated records shouldn't be displayed. Of course (and that justifies your [extension] method efforts) you don't want to add where IsActive to each and every query.

But extension or instance methods supplying a pre-filtered set aren't enough either! Any old time you apply an Include you'll be in trouble.

var projects = db.GetProjects.Include(p => p.Members);

Of course this includes all members, active or not. And (grief), Includes can't be filtered.

Fortunately, there are better solutions for this. The most viable one, in my opinion, is EntityFramework.DynamicFilters, by which you can define filters that will be applied any time a type is queried, also in Includes.

A very technical, but more complete, solution is the one proposed by the EF team's product owner himself, the code and explanation of which is available here. More complete, because it also redirects deletes to setting a delete flag instead of physical deletes.

Another, pretty elegant, solution, also redirecting deletes, is here. But it uses inheritance, so it may not play nice with other inheritance schemes.

\$\endgroup\$
  • \$\begingroup\$ By the way, the image is from here. \$\endgroup\$ – Gert Arnold Mar 31 '16 at 20:00
  • \$\begingroup\$ Wow, thank you. A very thoughtful and complete response. It gives me a lot to look into. I have just been running into the .Include being unfiltered issue. \$\endgroup\$ – Mark Kenyon Mar 31 '16 at 21:05

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.