Dependency injection in constructor vs Supplying argument in utility functions

Question

I have the following:

public class Person{
    public string FirstName {get; set;} = "";
    public string MiddleName {get; set;} = "";
    public string LastName {get; set;} = "";
    public string Title {get; set;} = "";
}

public class Program
{
    public static void Main()
    {
        var person = new Person {FirstName = "Daniel", MiddleName = "W", LastName = "Craig", Title = "Mr"};
    }
}

I am writing a service class that reports back a user's

• Full name (First + Middle + Last names)
• Title + Full name
• First name and last name

Using dependency injection, I have the following:

(1)

public class NameService{

    private readonly Person _person;    
    public NameService(Person person){
        _person = person;
    }

    public string GetFullName(){
        var fullName = $"{_person.FirstName} {_person.MiddleName} {_person.LastName}";

        return fullName;
    }

    public string GetFullNameWithTitle(){
        var fullNameWithTitle = $"{_person.Title} {_person.FirstName} {_person.MiddleName} {_person.LastName}";

        return fullNameWithTitle;
    }

    public string GetFirstLastName(){
        var fullName = $"{_person.FirstName} {_person.LastName}";

        return fullName;
    }
}

Which I can use like this:

var person = new Person {FirstName = "Daniel", MiddleName = "W", LastName = "Craig", Title = "Mr"};
var service = new NameService(person);

var fullName = service.GetFullName();

However, I can also define the service like this:

(2)

public class NameService
{
    public NameService(){}

    public string GetFullName(Person person)
    {
        var fullName = $"{person.FirstName} {person.MiddleName} {person.LastName}";
        return fullName;
    }

    public string GetFullNameWithTitle(Person person)
    {
        var fullNameWithTitle = $"{person.Title} {person.FirstName} {person.MiddleName} {person.LastName}";
        return fullNameWithTitle;
    }

    public string GetFirstLastName(Person person)
    {
        var fullName = $"{person.FirstName} {person.LastName}";
        return fullName;
    }
}

And then get a person's full name like so:

    var person = new Person {FirstName = "Daniel", MiddleName = "W", LastName = "Craig", Title = "Mr"};
    var service = new NameService();

    var fullName = service.GetFullName(person);

Which is the better approach? My thoughts:

(1)

Looks like the service is designed on a "per Person" basis. This means calling another person's full name will require instantiating a new service with a new Person argument (and that's okay).

(2)

This is a lightweight constructor that allows different Person object to be supplied into GetFullName(person) to allow different rendition of full name.

What other pros and cons can you think of?

Answer 1

If you use second option you should probably declare methods as static. Since you dont have any "data" associated with NameService object instance(service).

Like this:

public static string GetFullName(Person person)
{
    var fullName = $"{person.FirstName} {person.MiddleName} {person.LastName}";
    return fullName;
}

And then you can call...

NameService.GetFullName(person);

... without object initialazing - without line var service=new NameService();

Answer 2

Another option:

I always like to keep the methods and as close to the domain model as possible. This way you don't have to go looking for (or remember) a service which can do the naming for you.

In this solution you'll see a GetName method on the Person class itself with an option to supply how you want to have the name formatted.

Now after constructing a Person instance, you don't need to go looking for a piece of code which can do the naming for you, the method is there, you just need to go and find implementations of the INamingFormatter class and choose the one you need.

public class Person
{
    public string FirstName { get; private set; }
    public string MiddleName { get; private set; }
    public string LastName { get; private set; }
    public string Title { get; private set; }

    public Person(string title, string firstName, string middleName, string lastName)
    {
        Title = title;
        FirstName = firstName;
        MiddleName = middleName;
        LastName = lastName;
    }

    public Person(string firstName, string middleName, string lastName)
        : this(null, firstName, middleName, lastName)
    { }

    public string GetName(INamingFormatter namingFormatter)
    {
        return namingFormatter.Format(this);
    }
}

Interface:

public interface INamingFormatter
{
    string Format(Person person);
}

Implementation:

public class FullNameFormatter : INamingFormatter
{
    public static FullNameFormatter Instance = new FullNameFormatter();

    public string Format(Person person) => $"{person.FirstName} {person.MiddleName} {person.LastName}";
}

Call:

class Program
{
    static void Main(string[] args)
    {
        var person = new Person("Mr", "Danial", "W", "Craig");

        var name = person.GetName(FullNameFormatter.Instance);

        Console.WriteLine(name);

        Console.WriteLine("Hello World!");
    }
}

Answer 3

There is no reason for DI here because Person is not a service. It doesn't provide any functionality that you need to do other things. It's the data that the APIs are build for.

This means that you should choose the 2nd version with their pure methods.

---

However, there is a 3rd option...

You can split all three methods into their own services and have:

public class FullNameService : INameService { ... }
public class FullNameWithTitleService : INameService { ... }
public class FirstLastNameService : INameService { ... }

where all implement an interface like this one

public interface INameService
{
    string CreateName(Person person);
}

With such a design you would have even more freedom in adding and testing new services and they wouldn't affect each other.

---

No matter which solution you pick, you should always have some abstraction for the service (abstract class or interface). Without abstraction DI is pretty pointless because you cannot easily exchange the service and use an improved, different or even a test version.