4

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?

1 Answer 1

10

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.

7
  • I am eternally grateful for your detailed and clear response. Testing is important for all of us, so I shall go with a dedicated NotesViewModelsMapper.cs class that I can interface and unit test. Have a great day and many thanks :)
    – J86
    Commented Sep 25, 2019 at 10:42
  • Damn, you had to go and ruin a good answer with the nonsense, "Static methods have one severe drawback: they are notoriously hard to test". Static methods should be pure and pure methods are trivial to test because they do not need mocking. It's only when static methods have side effects that issues occur.
    – David Arno
    Commented Sep 25, 2019 at 14:03
  • 2
    @DavidArno: The word "with" is missing (they are notoriously hard to test with). It's not so much the testability of the static method itself that's the issue, but the lack of mockability of this static method when testing other classes (which make use of the static method). Though the confusion would have been cleared up if you had read one sentence further. Damn, you had to go and ruin a valid comment by not reading the entire paragraph you were responding to.
    – Flater
    Commented Sep 25, 2019 at 14:05
  • 1
    statics absolutely can be mocked as long as they are injected in as Func<>s
    – jk.
    Commented Sep 25, 2019 at 15:48
  • 1
    "It's not so much the testability of the static method itself that's the issue, but the lack of mockability of this static method when testing other classes". The whole point of a well behaved static method is that it doesn't need mocking/substituting. So it simplifies tests of code that use them, rather than making them difficult to test with.
    – David Arno
    Commented Sep 26, 2019 at 8:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.