C# Best Practice DI Pattern?

Question

Is there a best-practice pattern I can/should use for this scenario? I need access to some DI services in the OnFinished() method:

public class Product 
{
 public int Quantity {get;set;}
}
public abstract class Job 
{
 public int Duration {get;set;}
 
 public object? Product {get;set;}
 public virtual void OnFinished() {}
 public void Tick() { // checks to see if Duration is met, then calls OnFinished if so. Tick() is called externally. } 
}
public class BuildProduct : Job
{
 public string Name {get;set;}
 public override object OnFinished()
 {
 var product = new Product();
 // need access to DI services here to populate product, for example:
 var service = ServiceActivator.GetService<ProductCounter>();
 product.Quantity = service.CountStuff();
 //return product;
 Product = product;
 }
}

Above feels dirty in regards to a DI based app, testing is more complicated and any use of ServiceActivator doesn't feel right.

Job.OnFinished is called by itself after Job is complete, self dependent. Its on a timer, when the Duration is met, OnFinished is called.

Jobs are instantiated normally: var job = new BuildProduct();.

• Greg Burghardt: Can you edit your question to explain more about the BuildProduct and Job classes? What calls OnFinish()? What initializes a BuildProduct object? Are you using a particular framework?
• Greg Burghardt: What prompts Job to call OnFinished? Can you post the code for the Job class?

Answer 1

This is the service locator pattern. It is now generally considered to be an antipattern.

The main issue with this pattern is that is obfuscates the dependencies of a given class. Instead, it is generally preferred to explicitly inject each individual dependency into the object's constructor, so that you have an accurate summary of exactly which dependencies the object needs.

Additionally, because you're not using an interface on your dependency (i.e. IProductCounter as opposed to ProductCounter), you're not able to mock the dependency; which also isn't great.

Here's my proposed solution:

public class BuildProduct : Job
{
 private readonly IProductCounter productCounter;
 public BuildProduct(IProductCounter productCounter)
 {
 this.productCounter = productCounter;
 }
 public override object OnFinished()
 {
 var product = new Product();
 product.Quantity = productCounter.CountStuff();
 return product;
 }
}

• mxmissile: Thanks, I was thinking job = entity, so injecting a service didnt smell right. But I guess Job is more of a service I should pull out of the DI (Or Factory) to begin with.