Could new syntax make extension methods DI-friendly?

[sab39]

I didn't put this in "language ideas" because I don't have a specific proposal, but if others agree it's a worthwhile use case to consider, that could lead to coming up with something worth proposing.

I've recently started working in a codebase that uses a lot of dependency injection, and one thing I've noticed is that it prevents using extension methods in the way that I'm used to, because the dependencies can't be injected into the static extension method. A quick google search suggests I'm far from the first to encounter this. A simple example:

public static class EmploymentExtensions
{
  public static Employer GetEmployer(this Person person) => throw new NotImplementedException();
  public static IEnumerable<Person> GetEmployees(this Employer employer) => throw new NotImplementedException();
}

If these methods require database access, and the database access is available through an injected service or EF context rather than from something static, there's no way to implement them or make them available in a convenient way, even if the calling code has all the services that would be needed.

The best idea I've had so far - and it's not great - would be to allow declaring non-static extension methods, and some way to do an instance-level equivalent of using to bring them into scope. Something like, if the EmploymentExtensions class had all the static modifiers removed...

public class MyService
{
  using private readonly EmploymentExtensions employmentExtensions; // 'using' modifier registers the object as providing extension methods
  public MyService(EmploymentExtensions employmentExtensions, ...)
  {
    this.employmentExtensions = employmentExtensions;
    ...
  }
  void doSomething(Employer employer)
  {
     // employer.GetEmployees() is equivalent to employmentExtensions.GetEmployees(employer)
     foreach (var person in employer.GetEmployees()) ...; 
  }
}

[TahirAhmadov]

My immediate thought is that extension methods should not be used here. I know it may seem "nice" to sprinkle all these helpful extension methods on Person and stuff, but that's an anti-pattern.

[sab39]

Can you elaborate? I'm not sure what the point to having extension methods in the language at all would be if not for extending objects with methods that are common and helpful but aren't there by default. And being able to chain several extension methods together often makes for much more readable code than could be done without them.

[TahirAhmadov]

I'm not against extension methods, per se. However data retrieval falls under "business logic" rubric - it should be a class serving that purpose. If you want to combine your "entity" and "business logic" in one, say, Person has the properties First and Last, as well as methods like Marry(Person spouse), that's fine (I'm not a big fan of that, either) - but then you can the GetEmployer() as an instance method, as well. My preference, however, is to have IPersons interface, and Persons business logic class, and those will have GetEmployer(Person) instance method.
Extension methods do not fit anywhere in this equation.
I guess the criterion for me would be: use an extension method if all the data you need is available from its parameters (including the this parameter); if you need anything else, extension methods aren't the solution.

[sab39]

Whether it needs anything else could easily be an implementation detail, though: maybe the original version of the Employer or Person classes was designed so that the related objects would be loaded from the related DB tables at the same time, so all the information necessary was available from the Person object itself, but later for performance reasons it was decided to load lazily instead. Now any extension methods on Person that depended on the information would have to be refactored, along with all code using them.

[TahirAhmadov]

I guess my point is, I approach it as, you define a consumable endpoint, be it REST or whatnot, and the implementation of that "contract" is an operation which operates on entities (like Person) from storage (using data access layer).

public class Persons : IPersons
{
  public Persons(IDataAccess dataAccess) => this._dataAccess = dataAccess;
  public async Task EmailEmployerBirthdayWishesAsync(Person personRef)
  {
    var person = await this._dataAccess.Query<Person>().Where(p=>p.Id == personRef.Id).Include(p=>p.Jobs).FirstAsync();
    var employer = person.Jobs.FirstOrDefault(j=>j.Active)?.Employer;
    if(employer != null) await this.EmailBirthdayWishesAsync(employer);
  }
}

The existence of Person extension methods which retrieve things like Employer is a code smell to me.

[HaloFour]

It's not an implementation detail if it's required to be passed as a parameter, though. To keep it an implementation detail you'd want the Person instance to track the Database instance from where it was loaded. Or you'd embed a lazy-implementation of ICollection<T> which internally tracks that Database instance and will resolve the relationship on demand, which is what EF does (or used to do?)

I'd also caution against such lazy loading. It's easy to accidentally find yourself in an n+1 situation, constantly dipping back into the database unexpectedly and resulting in a large amount of overhead. Having to explicitly request the relationship through a repository facade makes it very clear that you're about to do work, especially blocking I/O bound work (which should be async anyway).

[HaloFour]

This feels like it fits under "roles": #5498

[sab39]

How so?

[HaloFour]

Or possibly as "implicit parameters", if the goal is to be able to pass the Database instance as an argument to these extension methods: #5989

[sab39]

Implicit parameters seem like very much the kind of thing I'm talking about! Thanks for the pointer.