Why is it bad to map between a Model and a ViewModel in an extension method

Question

In a recent Pull Request (PR) of mine, a colleague suggested that it was a bad idea to map between a model and its view model via extension methods.

I asked why and he said:

It isn't how extension methods are supposed to be used, it is not in the spirit of extension methods.

CODE

I have a Note.cs model class, and I want to pass a NoteReadViewModel.cs to a view that will list the notes in a table.

So I must map from Note.cs to NoteReadViewModel.cs. Here is the extension method I wrote:

public static class NoteExtensions
{
  public static NoteReadViewModel ToNoteReadViewModel(this Note note)
  {
    NoteReadViewModel viewModel = new NoteReadViewModel();
    viewModel.Body = note.Body;
    return viewModel;
  }
}

And I my NoteController.cs I was using it like this:

parentViewModel.HelpfulNote = _noteRepository
  .GetNoteBySectionId((short)Section)
  .ToNoteReadViewModel();

Is my colleague right? Can you explain it better? Why is it bad to use extension methods to do the mapping?

Answer 1

It isn't how extension methods are supposed to be used, it is not in the spirit of extension methods.

I'm going to hard counter that notion, because it is exactly the spirit of extension methods.

However, I do have to give your colleague one thing: static methods (which extension methods are) come with their own drawbacks. But this is unrelated to the argument that your colleague is making about the "spirit" of extension methods.

---

1. The spirit of extension methods

var myViewModel = myModel.ToMyViewModel()

Logically, this code can only exist in your web/UI application as it deals with the viewmodels. But since you have layer separation between models and viewmodels, that inherently means that this is not the assembly that contains your model's class definition.

In other words, without extension methods, you would be unable to add ToMyViewModel() as a behavior of your model class, because the assembly in which your model exists does not know what a viewmodel is (and it shouldn't know that).

Extension methods allow you to extend a pre-existing class definition (from another assembly) to add method-like handling to the type. That is exactly why extension methods exist, and that is exactly what you are trying to achieve in your code.

---

2. On a technical level

On a technical level, there is no difference between a static method and an extension method:

public static Foo MyStaticMethod(Bar obj)
{
    return new Foo()
    {
        Name = obj.Name
    };
}

public static Foo MyExtensionMethod(this Bar obj)
{
    return new Foo()
    {
        Name = obj.Name
    };
}

The only difference between them is syntactic sugar in how you call it:

var myFoo = MyClass.MyStaticMethod(myBar);

var myFoo = myBar.MyExtensionMethod();

This is the only real difference. And there is no technical difference between them, the compiler will treat them the same way.

The only argument here is one of style and readability, and I cannot agree with the idea that extension methods are somehow inferior to static methods in this regard.

Let's look at your example for a real world exercise. Which do you (and your colleague) find the most readable:

var vm = _noteRepository
                 .GetNoteBySectionId((short)Section)
                 .ToNoteReadViewModel();

var vm = NoteMapper.ToNoteReadViewModel(
             _noteRepository
                 .GetNoteBySectionId((short)Section)
         );

The first example stick to a chronological order of operations. When reading it, you parse it in the order that it's executed, which makes sense.

The second example puts the last algorithm (the mapping) as the first thing to read. This inverted model ("stack-like") requires developers who read "backwards" and is thus less intuitive and less readable.

This becomes even more egregious when you are using method chaining for everything but the static method:

    var vm = NoteMapper.ToNoteReadViewModel(
             _noteRepository
                 .Set(...)
                 .Where(...)
                 .Select(...)
                 .ToList()
         );

Note: I'm not advocating manual query building, I'm just giving a basic example of chained methods.

In this case, every method is in chronological order, except for that static method. Had you used an extension method, it would've been much more readable:

    var vm = _noteRepository
                 .Set(...)
                 .Where(...)
                 .Select(...)
                 .ToList()
                 .ToNoteReadViewModel();

Extension methods look and feel like a class method, which is exactly why they were created; to enable developers to have that look and feel even though the method isn't part of the actual class definition.

---

3. The drawback of static methods

Static methods have one severe drawback: they are notoriously hard to test with. They don't allow for mocking/dependency injection, and this can be an obstacle.

If your mappers are supposed to be built as components so that you can easily swap them out (whether for unit testing purposes or changing the actual behavior during runtime), then statics are not the way to go.

However, your colleagues argument is about the intention of extension methods, and he's dead wrong about that. It is exactly the spirit that extension methods provide to the developer.