Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idea: Support an instance of the target as argument to Map method #14

Open
marceln-gh opened this issue Dec 7, 2021 · 3 comments
Open
Assignees

Comments

@marceln-gh
Copy link

marceln-gh commented Dec 7, 2021

Hi,

I like this library.

Since AutoMapper should really only be used for certain use cases it might not always be a good idea to use AutoMapper.
Using this library I can still create mappings that don't clutter my code by keeping them separate from my logic.

In some cases I would like to provide a target instance that is already available to me instead of letting the mapping create a new instance of the object. Would it be possible to add support for this?

I've forked this repository and played around with it and I think it's possible to support his scenario.
At the moment I've separated the functionality in a different interface because the covariant generic type (TTarget) cannot be used as an argument of a method.

I've not created a pull-request because you might not want to add a separate interface for this and I was only playing around.
If you'd like we could further discuss this feature request and I can provide a pull-request that's more in line with your vision.

You can take a look at my fork over here: https://github.com/marceln-gh/mapr/tree/feature/request-14
(branch: feature/request-14)

@rena0157
Copy link
Owner

rena0157 commented Dec 8, 2021

Hey there,

Thank you very much for the interest and suggested feature!

I took a look at the branch that you have created and am trying to create any example use case of what you are describing to wrap my head around it. Is this what you are thinking:

  // Model to be mapped from
  public record Person(string Name, int Age);

  // Model to be mapped to
  public record PersonModel
  {
      public string Name { get; set; }
      
      public int Age { get; set; }
  }

  // The map
  public class ComplexPersonMap : IComplexMap<Person, PersonModel>
  {
      // As per usual implementation..
      public PersonModel Map(Person source)
      {
          throw new NotImplementedException();
      }

      // The implementation specific to the complex map
      public PersonModel Map(Person source, PersonModel target)
      {
          target.Age = source.Age;
          target.Name = source.Name;
          
          return target;
      }
  }

Which then could be used like:

IMapper mapper = new Mapper(...);

var person = new Person("John", 42);
var personModel = new PersonModel();

var mappedPersonModel = mapper.Map(person, personModel);

It does look like it could be implemented based on what you have put together. Off the bat my only concern is that it could potentially increase the possibility of runtime errors if an IComplexMap is not registered and that map method is called.

Out of curiosity, what is the reason for your preference to have the initialized object passed into the map instead of the map initializing the object?

@marceln-gh
Copy link
Author

Hi,

In the past I've come across situations where I already have an instance of a class but it's not complete yet. Then I would like to fill the void using a mapping (from a different source to an existing target). This would avoid allocating objects that are not needed.

AutoMapper and Mapster both support this scenario so it would be great if your library could support it as well.

Maybe the IComplexMap interface is not the way you would implement this in your library.
But since you've explicitly chosen to make TTarget covariant it's not possible to add the new method to the existing IMap interface.

@rena0157
Copy link
Owner

rena0157 commented Dec 8, 2021

I see what you are saying. I have not run into that use case yet, but I see how it could be useful. I will play around with this a little, see what I can do to implement something that supports this.

@rena0157 rena0157 self-assigned this Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants