In general, when you write something here, try to imagine what a potential user would like to read. Suppose you don't know anything about this tool, and this page is going to be the one chance the tool has for you to decide whether to use it or not.
I don't like <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion>. I think a lot of people are moving to dotnet core, so this thing should work on that as well, unless you are using some specific features from the .NET framework v4.7.2 that you need. Support at least dotnet 6 (i.e. net6.0)
About it
Change name to "About Mapper" or something. "About it" is not very professional.
The readme section concerning TDestination Map<TDestination>(IEnumerable<object> source) states:
both destination type and source type must be types that are assignable from IEnumerable (e.g. List, Array).
But isn't it the case that the source type must be assignable toIEnumerable<object>? Otherwise you wouldn't be able to call the method.
In any case, the following signature seems more reasonable to me:
This way your mapper just maps the items that are enumerated, and is agnostic to the means of enumeration (and hence the way they are enumerated back). Then a user who wants a List<TDestination> can just use ToList. Notice how the signature more closely matches the one there, and that this is an extension method. Then we could have:
var aList = new List<A>() { 1, 2, 3 };
var bList = intList.Map<B>();
I disagree with the interface that if you pass null as the destination object you get a newly created object back. It's a bit awkward, I don't think it will work nicely with the new nullable reference types, but more importantly it enables this pretty serious bug:
B b = null;
try {
// do some stuff
// now I think that b is not null
Map(a, b); // you create a new object and return it to nothing
} catch(Exception e) {
// handle exceptions
}
// b is still null, now we have the following bug
Console.WriteLine(b.ToString()); // unhandled exception! WTF!
Configurationless mapping
I'll put some examples of what your tool should be capable of in the UnitTests
For each of these different behaviors, please provide an example. If that's too much content, then make a few examples that demonstrate all the behavior.
You need to come up with something better for "cyclic data structures." You need a better explanation of the problem you're talking about, the solution you've chosen and why, and then how users can alter this behavior. In general we should use some reasonable defaults, and 5 does not seem reasonable for a maxDepth except for when testing or developing the project.
Things you should know about when using mapper
It sounds like I can't map a property of type Derived to a property of Base. Is there a reason for that? Demanding strict type equality for every property seems to be an unusually strict requirement for users, why wouldn't you at least try to recursively use your own mapper? Or at least offer a RecursiveMap function that implements this behavior?
The last two bullets here present some serious philosophical issues. On the one hand, you are going with "immutability" as a priority by not copying references between objects. Okay, but this is a really important part of the way you do things. It explains why you want a default constructor for everything. Fine. But then you say If your destination property does not have a public set method ... destination's property value will not change - but not having a public set method is an excellent and popular way to enforce immutability! This doesn't make sense, you need to figure it out.
Motivation and automapper comparison
This whole paragraph is a good start, but needs to be improved in general. I don't care if you use chatgpt or something to help you make it better.
"sometimes it works very randomly" - provide an example (especially since your criticising some other tool)
Talking about "with configuration set" is a bit convoluted. It would probably be better to say something like "this tool works more intuitively than AutoMapper by default". But this is a claim, and you should provide some evidence.
The iteration is straightforward, but the rules in the previous paragraph are not so. In fact, they are extremely restrictive about how the tool can be used.
Having code that is easy to read for beginners is not an acceptable design goal for a project. Having short source code so that it can be copy/pasted easily into another project might be, if it's somehow challenging (for example, having source code that works with different language versions or IDE's might present some interesting challenges...)
Better explain the issue about properties being ignored that you are attempting to improve upon. This is probably the most relevant bit in the paragraph, but you just state "deep" in quotes and say "I will try to make it without these issues." No, you need to have a number of non-trivial tests demonstrating that AutoMapper has this issue and yours doesn't.
From README.md
<TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion>
. I think a lot of people are moving to dotnet core, so this thing should work on that as well, unless you are using some specific features from the .NET framework v4.7.2 that you need. Support at least dotnet 6 (i.e.net6.0
)About it
Mapper
" or something. "About it" is not very professional.TDestination Map<TDestination>(IEnumerable<object> source)
states:But isn't it the case that the source type must be assignable to
IEnumerable<object>
? Otherwise you wouldn't be able to call the method.IEnumerable<TDestination> Map<TSource, TDestination>(this IEnumerable<TSource> source)
This way your mapper just maps the items that are enumerated, and is agnostic to the means of enumeration (and hence the way they are enumerated back). Then a user who wants a
List<TDestination>
can just useToList
. Notice how the signature more closely matches the one there, and that this is an extension method. Then we could have:null
as the destination object you get a newly created object back. It's a bit awkward, I don't think it will work nicely with the new nullable reference types, but more importantly it enables this pretty serious bug:Configurationless mapping
Things you should know about when using mapper
Derived
to a property ofBase
. Is there a reason for that? Demanding strict type equality for every property seems to be an unusually strict requirement for users, why wouldn't you at least try to recursively use your own mapper? Or at least offer aRecursiveMap
function that implements this behavior?If your destination property does not have a public set method ... destination's property value will not change
- but not having a public set method is an excellent and popular way to enforce immutability! This doesn't make sense, you need to figure it out.Motivation and automapper comparison
"deep"
in quotes and say "I will try to make it without these issues." No, you need to have a number of non-trivial tests demonstrating that AutoMapper has this issue and yours doesn't.