MapsterMapper
Adapt(source, destination) does not work when destination is a C# record
When using Mapster to map data into an existing object, the mapping works correctly if the destination is a class, but does nothing when the destination is a C# record.
This appears to be caused by record immutability, but the current behavior is not clearly documented and can be misleading.
in 7.4.0 Records in C# is not supported and record type from mapster not equal Records in C# ;
The latest pre-release versions have Records support.
var result = source.Adapt(destinationRecord);
it using "with like modify" and you data should be in the result variable.
@DocSvartz
When using the overload to modify a record, the record is not changed because records are immutable.
public static TDestination Adapt<TSource, TDestination>(this TSource source, TDestination destination)
Do you think we should just ignore this, or at least throw an error explaining that immutable types cannot be modified? Alternatively, should it be handled by passing the destination by ref?
@roohial57 Do you expect Records to behave immutably? In Mapster record types were originally intended to be immutable. When I fixed the problem #537, I save this behavior.
Do you think we should just ignore this, or at least throw an error explaining that immutable types cannot be modified?
In my opinion, we must write this information about different behavior for Records in Wiki. @andrerav @DevTKSS @stagep What do you think about it?
Alternatively, should it be handled by passing the destination by ref?
What do you mean? This behavior has nothing to do with how the destination is transmitted.
In my opinion, the signature
public static TDestination Adapt<TSource, TDestination>(this TSource source, TDestination destination)
implies that the data from source is be mapped onto destination.
When destination is immutable and this method is called, no error is thrown to indicate that the mapping did not occur.
As a result, the user may assume the data was mapped successfully, while in fact no mapping actually took place.
And this misleads the user.
@DocSvartz , Of course, your perspective is more comprehensive and inclusive than mine.
@DocSvartz This should be added to the documentation. I will put in some time over the weekend to review the issues where I need to respond. I have to understand what functionality docfx provides but I am sure it has something to handle documentation that is in review and not ready to be published. I am just not sure how granular this control is.
@roohial57 if I asked you how many ones can you put into zero, would you answer zero, or would you answer that this is a mathematical error. I would answer zero because that is the truth. So how many properties can you change on an immutable object? Zero. A mapper can only do what the destination allows it to do. So it was successful at mapping, it just mapped zero properties. If you map between 2 class instances and the classes have many properties but only one property that is mapped because it is of the same type and name, is it up to Mapster to tell you how many properties were mapped? I don't want to open a can of worms but I do see value from having an overload such as
public static TDestination Adapt<TSource, TDestination>(this TSource source, TDestination destination, out int result)
but this could be a long term goal if others see value in this. And if value was seen in this, then it would make an excellent first contribution, something I would consider doing myself. TBH, it sounds fairly rote and something that AI could probably do a large portion of. As long as it broke nothing and had excellent test coverage then it sounds fairly straightforward because you are simply ensuring the chain of results are passed up the chain to the final public method.
Also, the result would not have to be just an int, it could be a MappingResult type that has more than one property,
@stagep
Your idea is also Correct.
If the destination is immutable, what does the following line do, and what problem would arise if it were removed?
source.Adapt(destination);
When a line of code does nothing and its presence or absence makes no difference, its existence creates ambiguity.
@stagep By the way, I really like the idea of MappingResult.
@roohial57 I agree with you 100% that there is ambiguity. It is like asking what is zero divided by zero. In mathematics, we say this is indeterminate. In the non mathematical world we would say it is ambiguous. I remember as a child having a physical calculator that would give you the result of 1 but it would flash 1. So it was ambiguous, because it was giving you an answer and an error at the same time. Why 1, because any number divided by itself is one, but any number divided by zero is any number. If that is not ambiguity then I don't know what is ambiguity :) Really enjoying this discussion with you, very constructive. Thanks.
@stagep @roohial57 You are not taking into account the method signature, it does not return void :)
source.Adapt(destination); -> var result = source.Adapt(destination);
source.Adapt(destination);
This returns the mapping result.
And it seems to me that the result may be completely different depending on which Destination Type is used. 🤔
@DocSvartz @roohial57 I realize now that we should prioritize returning the expected result when mapping to an existing record, over introducing a Result concept. I see 3 options.
- Introduce an Exception as an interim fix
- Do nothing and correct the behavior of records
- Do both 1 and 2
Here is some simplified code that demonstrates the change in handling records
var source = new Source(Id: 1, Name: "Source");
var destination = new Destination(Id: 2, Name: "Destination");
Console.WriteLine($"Source: {source}");
var mappedR = Map(source, destination);
Console.WriteLine($"Map: {mappedR}");
mappedR = MapNew(source, destination);
Console.WriteLine($"MapNew: {mappedR}");
Console.ReadKey();
static Destination Map(Source source, Destination destination)
{
return destination;
}
static Destination MapNew(Source source, Destination destination)
{
return destination with
{
Id = source.Id,
Name = source.Name
};
}
public record Source(int Id, string Name);
public record Destination(int Id, string Name);
The output is
Source: Source { Id = 1, Name = Source }
Map: Destination { Id = 2, Name = Destination }
MapNew: Destination { Id = 1, Name = Source }
I believe that MapNew returns the expected result. Am I correct?
Adapt is currently working with Records .
like this:
static Destination MapNew(Source source, Destination destination)
{
return destination with
{
Id = source.Id,
Name = source.Name
};
}
This is how it works now
when
public class Destination()
{
public int Id {get; set;}
string Name { get; set; };
};
static Destination Map(Source source, Destination destination)
{
destination.id = source.id
destination.Name = source.Name
return destination;
}
@stagep This is if we leave the concept of immutable types
// to mutable type
public static void Adapt<TSource, TDestination>(this TSource source, TDestination destination)
{
if (typeof(TDestination).IsImmutable())
throw new Exception($"{typeof(TDestination).Name} is Immutable Using MapToTarget");
Adapt(source, destination, TypeAdapterConfig.GlobalSettings);
}
// to immutable type
public static TDestination MapToTarget<TSource, TDestination>(this TSource source, TDestination destination)
{
return Adapt(source, destination, TypeAdapterConfig.GlobalSettings);
}
@DocSvartz You are absolutely right. I had not fully understood the correct way to use this method. I apologize for taking up your time.
@roohial57 @stagep I'm not a fan of ambiguity either :) I think we could supported two different methods, as I like indicated here. What do you think about this? Is this an optimal compromise?
@DocSvartz Although the ambiguity has largely been resolved for me, I still welcome the suggestion of having two methods.
@DocSvartz sorry I didn't seen this notification.
My network is broken today so I only can answer via mobile.
I am not sure if I understood the now targeted solution correctly can you maybe explain it to me?
I know Immutability from Mvux in Uno Platform development and they have some cool extension methods in place 🤔
Maybe there would be one to replicate for this?
Like something like that:
source.Create<T>() where it would be genius if we could anyhow integrate dto source gen into this, which is where I see the biggest lack of capability by now. We either can have dto creation or auto mappings but trouble using both at the same time.
@DevTKSS
I am not sure if I understood the now targeted solution correctly can you maybe explain it to me?
Should the current public static TDestination Adapt<TSource, TDestination>(this TSource source, TDestination destination)
be split into two separate methods capable of updating only mutable types and only immutable types?
I know Immutability from Mvux in Uno Platform development I looked, mapster have the same approach about the immutability of Records.
Maybe there would be one to replicate for this? Like something like that: source.Create<T>() where it would be genius if we could anyhow integrate dto source gen into this, which is where I see the biggest lack of capability by now. We either can have dto creation or auto mappings but trouble using both at the same time.
Could you provide an example? As far as I understand, it uses its own source code generator to create the necessary magic :)
@DocSvartz @DevTKSS I believe that we simply have to decide that when we map (using map as the generic term for what Mapster does) and we map to a record, should we take the approach that records are immutable so you get back the same record, an exception is thrown, or we return a new record with the new values. Here is an AI summary when asked about this situation:
Mapping Library Behavior - when mapping from a class instance (mutable) to a record (immutable), the library has two choices:
Option 1
- Provide a new record with updated values
- This respects immutability
- It aligns with developer expectations: mapping means “produce a mapped representation,” not “mutate the target.”
- It avoids runtime errors and makes the library more usable
Option 2
- Throw an exception because the record is immutable
- This is technically correct (records can’t be mutated), but it’s hostile to developer ergonomics
- It forces the caller to handle the mapping manually, defeating the purpose of the library
- Exceptions should be reserved for unexpected or invalid states, not for normal usage
When I read the discussion about two separate methods, I see a code smell. In what scenario would someone mapping to an existing record want it to return the destination record and/or throw an exception? Returning a new record with the new values seems like the logical choice. If I am missing a use case where this behavior it is not logical, then please give me an example,
@stagep I follow the Option 1 approach.
and i propose a redesign current method:
If a method is intended only for mutable entities, it should return nothing (void), and only mutate the destination. And then it can't work with immutable entities and must throw an exception. You can't change immutable.
public static void Adapt<TSource, TDestination>(this TSource source, TDestination destination)
{
if (typeof(TDestination).IsImmutable())
throw new Exception($"{typeof(TDestination).Name} is Immutable Using MapToTarget");
Adapt(source, destination, TypeAdapterConfig.GlobalSettings);
}
And adding a new method only for immutable types in the concept of option 1
Since the concept of the current universal method
public static TDestination Adapt<TSource, TDestination>(this TSource source, TDestination destination)
raises questions about what will be the result of such a method depending on the mutability or immutability of the destination parameter type.
@stagep Or maybe the Wiki description just needs to be changed. If this is just a misconception due to the documentation being out of date :) How I originally classified this issue.
I 100% agree and missed the void versus return type method differentiation. The void method cannot mutate, the return method returns a new record with "updated" values. We will make this the first update to the documentation. But an update, as in, first lets get what we have into docfx, commit, then do a docs issue update / review / commit. I see this has the label "need Docs modify" already. Good work.
@stagep If I understand correctly, you're in favor of the proposed separation concept. If yes, please mark this comment 👍
@DocSvartz @DevTKSS I believe that we simply have to decide that when we map (using map as the generic term for what Mapster does) and we map to a record, should we take the approach that records are immutable so you get back the same record, an exception is thrown, or we return a new record with the new values. Here is an AI summary when asked about this situation:
Mapping Library Behavior - when mapping from a class instance (mutable) to a record (immutable), the library has two choices:
Option 1
- Provide a new record with updated values
- This respects immutability
- It aligns with developer expectations: mapping means “produce a mapped representation,” not “mutate the target.”
- It avoids runtime errors and makes the library more usable
Option 2
- Throw an exception because the record is immutable
- This is technically correct (records can’t be mutated), but it’s hostile to developer ergonomics
- It forces the caller to handle the mapping manually, defeating the purpose of the library
- Exceptions should be reserved for unexpected or invalid states, not for normal usage
When I read the discussion about two separate methods, I see a code smell. In what scenario would someone mapping to an existing record want it to return the destination record and/or throw an exception? Returning a new record with the new values seems like the logical choice. If I am missing a use case where this behavior it is not logical, then please give me an example,
I would see record mapping coming from the MVUX approach which is essentially statefull management with immutability as similar mapped to this:
source.AdaptAsRecord<SourceTypeDto>() which should defintly not throw!
it should instead create a immutable record of the source type like: new SourceTypeDto with { Property1 = source.Property1 } and so on. Or fully immutable by using the ctor parameters!
@DevTKSS the method that has no return type / returns void (however you look at it) has to throw an exception. The method that returns the type (the destination type) returns the new record with the new values as in your example. Understand?
@stagep but why doesn't it has a return type? 🤔 Am I missing the overload or method that would be returning that what am thinking of?
@DevTKSS
but why doesn't it has a return type? 🤔 To clearly indicate the expected result.
Describe what is happening in these code examples. And what do we expect to get when we call each of these methods.
// public static void WriteLine (string? value);;
var string = "Hello";
Console.WriteLine(string);
// public Uri MakeRelativeUri(Uri uri);
Uri address1 = new Uri("http://www.contoso.com/");
Uri address2 = new Uri("http://www.contoso.com/index.htm?date=today")
address1.MakeRelativeUri(address2); // Where will the result be that I expect to get from executing this method?
(address1 or address2 or in new instanse Url which will be returned method )
@DocSvartz @stagep
I think that using suitable parameter names can remove ambiguity.
In the following method, based on the name of the second parameter (destination), it is expected that this parameter represents the result of the method:
public static TDestination Adapt<TSource, TDestination>(this TSource source, TDestination destination)
However, this signature suggests that the second parameter (baseValue) is merely base data used by the method and is not the method result:
public static TDestination Adapt<TSource, TDestination>(this TSource source, TDestination baseValue)