C#: Helper Method Dependency Injection within Constructor

Question

I have a IData interface that handles the SQL data access logic and as well as retrieving data from SQL Server:

public interface IData
{
  Task<IEnumerable<string>> GetUsers();
}

I have a Emailer class that sends an email with the list of Users and currently I am injecting IData via the constructor and accessing the GetUsers() method within another Method of the Emailer class:

public class Emailer
{
  private readonly IData _data;

  public Emailer(IData data)
  {
    _data = data;
  }

  public async Task Send()
  {
    var users = await _data.GetUser(); // access data here
  }
} 

In an act to try and refactor this code and apply the Single Responsibility principle, I want to abstract away the GetUser() logic into its own seperate class however how can I do this without having to initialise IData in the constructor of the new class to avoid having to pass the IData object as a parameter when newing up the class?

Example:

public class UserData
{
  private readonly IData _data;

  public UserData(IData data)
  {
    _data = data; // not ideal as I'd have to pass the object from the Emailer class and other classes that call Emailer
  }

  public async Task Fetch()
  {
    var users = await _data.GetUser(); // access data here
  }

  public async Task ManipulateUser(){} // example of future extensibility hence the reason for a seperate class
} 

Could implementing a factory pattern to solve this problem (if yes, how?) or any other ideas?

TIA

Answer 1

Can't you just abstact bind UserData to an interface, called IUserData and inject that into your Emailer class if needed?

I'm on my mobile right now but if you like I can post a code example later if you don't understand.

--UPDATE--

Here is the updated code.

//Refactor your existing classes to look like this.
public interface IData
{
    Task<IEnumerable<string>> GetUsers();
}

public interface IEmailer
{
    Task Send();
}

public class Emailer : IEmailer
{
    private readonly IUserData _userData;

    public Emailer(IUserData userData)
    {
        _userData = userData;
    }

    public async Task Send()
    {
        var users = await _userData.Fetch(); // access data here
    }
}
public interface IUserData
{
    Task Fetch();

    Task ManipulateUser();
}

public class UserData : IUserData
{
    private readonly IData _data;

    public UserData(IData data)
    {
        _data = data; 
    }

    public async Task Fetch()
    {
        var users = await _data.GetUser();
    }

    public async Task ManipulateUser() { } 
}

//Example of class calling Emailer
public interface IRandomClass
{
    Task RunSomeCode();
}

public class RandomClass : IRandomClass
{
    private readonly IEmailer _emailer;
    public RandomClass(IEmailer emailer)
    {
        _emailer = emailer;
    }

    public async Task RunSomeCode()
    {
        //Example code
        await _emailer.Send();
    }
}

Binding the dependencies (assuming you're using .netcore)

        services.AddScoped<IUserData, UserData>();
        services.AddScoped<IEmailer, Emailer>();
        services.AddScoped<IRandomClass, RandomClass>();

This should give you a good baseline to work from.

Let me know if you need anything else.

Happy coding!

Answer 2

Compare interfaces to black boxes. They do stuff, but you have no idea of how they do it.

But, if you look at the interface name and have no idea of what they are doing, you got it wrong.

In this case, IData seems like a database god object. Am I correct? Does it handle all your SQL operations? If so, divide it into different interfaces which all have a clear responsibility. "I handle DB access for user objects" = IUserData.

Regarding methods, C# is an object-oriented language, and classes (and interfaces) are the primary way of letting objects communicate. You won't get any benefits in this case if you try to inject methods instead.

Update

Do not refractor code to remove dependencies. composition makes the code more robust. Instead, use a Inversion Of Control container to create your classes.

Read this article to get started: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-6.0

Answer 3

You seem to redesign a class prematurely violating the YAGNI principle. If you aren't extending UserData right now, there is no need to introduce a further abstraction that does nothing more than the code already did.

When you do have extra responsibilities then you will create the UserData object passing IData into its constructor. Then you pass the UserData instance into the Emailer constructor. You basically switch one object for another.

Answer 4

IData seems too generic for this use case. You can instead introduce IUserData which would be only responsible for handling data access for the User.

Then your setup will look like this:

public interface IUserData
{
  Task<IEnumerable<string>> GetUsers();
}

public class UserData: IUserData
{
  public async Task<IEnumerable<string>> GetUsers()
  { ... }
}

public interface IEmailer
{
  Task Send();
}

public class Emailer : IEmailer
{
  private readonly IUserData _userData;

  public Emailer(IUserData userData)
  {
    _userData = userData;
  }

  public async Task Send()
  {
    var users = await _userdata.GetUser();
  }
} 

Alternatively, you can have 3 layers with a generic Data class in charge of all the data access operations and have the UserData class fine-grain it. This of course depends on how large and unmanageable can this data service become. I personally prefer to have tailor-made services for each data access use case.