Best practices regarding type mapping and extension methods

Question

I want to ask some questions about best practices regarding mapping types and using extension methods in C#. I know this topic has been discussed multiple times over past few years, but I've read a lot of posts and still have doubts.

The problem I encountered was extending class that I own with "convert" functionality. Let's say that I have class "Person" that represents an object which will be used by some logic. I also have a class "Customer" that represents a response from external API (actually there will be more than one API, so I need to map each API's response to common type: Person). I have access to both classes' source code and theoretically can implement my own methods there. I need to convert Customer to Person so I can save it to database. The project doesn't use any automatic mappers.

I have 4 possible solutions in mind:

1. .ToPerson() method in Consumer class. It's simple, but it seems like breaking the Single Responsibility pattern to me, especially that the Consumer class is mapped to other classes (some required by another external API) as well, so it would need to contain multiple mapping methods.

2. Mapping constructor in Person class taking Consumer as an argument. Also easy and also seems like breaking Single Responsibility pattern. I'd need to have multiple mapping constructors (since there will be class from another API, providing the same data as Consumer but in slightly different format)

3. Converters class with extension methods. This way I can write .ToPerson() method for Consumer class and when another API is introduced with it's own NewConsumer class, I can just write another extension method and keep it all in the same file. I've heard an opinion that extension methods are evil in general and should be used only if absolutely necessary so that's what is holding me back. Otherwise I like this solution

4. Converter/Mapper class. I create separate class that will handle conversions and implement methods that will take source class instance as an argument and return destination class instance.

To sum up, my problem can be reduced to number of questions (all in context to what I described above):

1. Is putting conversion method inside (POCO?) object (like .ToPerson() method in Consumer class) considered breaking single responsibility pattern?

2. Is using converting constructors in (DTO-like) class considered breaking single responsibility pattern? Especially if such class can be converted from multiple source types, so multiple converting constructors would be required?

3. Is using extension methods while having access to original class source code considered bad practice? Can such behavior be used as viable pattern for separating logic or is it an anti-pattern?

Answer 1

Is putting conversion method inside (POCO?) object (like .ToPerson() method in Consumer class) considered breaking single responsibility pattern?

Yes, because conversion is another responsibility.

Is using converting constructors in (DTO-like) class considered breaking single responsibility pattern? Especially if such class can be converted from multiple source types, so multiple converting constructors would be required?

Yes, conversion is another responsibility. It makes no difference if you do it via constructors or via conversion methods (e.g. ToPerson).

Is using extension methods while having access to original class source code considered bad practice?

Not necessarily. You can create extension methods even if you have the source code of the class that you want to extend. I think that whether you create an extension method or not should be determined by the nature of such method. For example, does it contain a lot of logic? Does it depend on anything other that the members of the object itself? I would say that you shouldn't have an extension method that requires dependencies to work or that contains complex logic. Only the simplest of logic should be contained in an extension method.

Can such behavior be used as viable pattern for separating logic or is it an anti-pattern?

If the logic is complex, then I think that you shouldn't use an extension method. As I noted earlier, you should use extension methods only for the simplest things. I wouldn't consider conversion simple.

I suggest that you create conversion services. You can have a single generic interface for it like this:

public interface IConverter<TSource,TDestination>
{
    TDestination Convert(TSource source_object);
}

And you can have converters like this:

public class PersonToCustomerConverter : IConverter<Person,Customer>
{
    public Customer Convert(Person source_object)
    {
        //Do the conversion here. Note that you can have dependencies injected to this class
    }
}

And you can use Dependency Injection to inject a converter (e.g. IConverter<Person,Customer>) to any class that requires the ability to convert between Person and Customer.

Answer 2

Is putting conversion method inside (POCO?) object (like .ToPerson() method in Consumer class) considered breaking single responsibility pattern?

Yes. A Consumer class is responsible for holding the data related to a consumer (and possibly performing some actions) and should not be responsible for converting itself into another, unrelated type.

Is using converting constructors in (DTO-like) class considered breaking single responsibility pattern? Especially if such class can be converted from multiple source types, so multiple converting constructors would be required?

Probably. I typically have methods outside both the domain and DTO objects to do the conversion. I often use a repository that takes in domain objects and (de)serializes them, either to a database, memory, file, whatever. If I put that logic in my domain classes, now I've tied them to one particular form of serialization, which is bad for testing (and other things). If I put the logic in my DTO classes, then I've tied them to my domain, again limiting testing.

Is using extension methods while having access to original class source code considered bad practice?

Not in general, although they can be overused. With extension methods, you create optional extensions that make code easier to read. They do have drawbacks - it;s easier to create ambiguities that the compiler has to resolve (sometimes silently) and can make code more difficult to debug since it's not entirely obvious where the extension method is coming from.

In your case, a converter or mapper class seems like the simplest, most straightforward approach, although I don't think you're doing anything wrong by using extension methods.

Answer 3

I know this is old, but is still revelent. This will allow you to convert both ways. I find this helpful when working with Entity Framework and creating viewmodels (DTO's).

public interface IConverter<TSource, TDestination>
{
    TDestination Convert(TSource source_object);
    TSource Convert(TDestination source_object);
}

public class PersonCustomerConverter : IConverter<Person, Customer>
{
    public Customer Convert(Person source_object)
    {
        //Do the conversion here. Note that you can have dependencies injected to this class
    }
    public Person Convert(Customer source_object)
    {
        //Do the conversion here. Note that you can have dependencies injected to this class
    }
}

I find AutoMapper to be a bit much for doing really simple mapping operation.

Answer 4

How about using AutoMapper or custom mapper could be used Like

MyMapper
   .CreateMap<Person>()
   .To<PersonViewModel>()
   .Map(p => p.Name, vm => vm.FirstName)
   .To<SomeDTO>()
   .Map(...);

from domain

 db.Persons
   .ToListAsync()
   .Map<PersonViewModel>();

Under the hood you could abstract AutoMapper or roll your own mapper