cezarypiatek / MappingGenerator

:arrows_counterclockwise: "AutoMapper" like, Roslyn based, code fix provider that allows to generate mapping code in design time.
https://marketplace.visualstudio.com/items?itemName=54748ff9-45fc-43c2-8ec5-cf7912bc3b84.mappinggenerator
MIT License
1.03k stars 120 forks source link

Clone object mapping #31

Closed phoenix172 closed 5 years ago

phoenix172 commented 6 years ago

It would be nice if there was a straightforward way to create a clone. I know that currently that can be accomplished by creating a mapping that takes and returns the same object, and after that alt-enter -> explicit mapping, but I was wondering if it wouldnt be better to directly generate explicit mapping in the above scenario. Writing a mapping method that takes and returns the same type doesn't really make sense in any other case, unless you want to clone (and maybe modify slightly) the passed object.

P.S: Love the tool. Saves me from writing boring code.

P.P.S: Sorry for overwhelming you with issues :D

cezarypiatek commented 6 years ago

Great idea, I like it. I will try to implement it.

cezarypiatek commented 6 years ago

Here is my first attempt of implementing refactoring that generate clone method. Would you mind to test it and give me some feedback about the results? Version with this experimental feature is available here

https://ci.appveyor.com/api/buildjobs/saey15a33qqlgh92/artifacts/MappingGenerator%2FMappingGenerator%2FMappingGenerator.Vsix%2Fbin%2FRelease%2FMappingGenerator.vsix

clone_sample

cezarypiatek commented 6 years ago

@phoenix172 are you still interested in this feature?

phoenix172 commented 6 years ago

Yeah, sorry been away for a while. Gonna test it when I get to work.

phoenix172 commented 6 years ago

Ok, I tested the new update. It seems good with a few notes:

  1. When you try to generate clone method for class that has a base type, the ICloneable interfaces gets placed before the base type, which generates a compile error. Manually fixing it is not difficult, but I think it would be quite easy for you to correct the behavior.

2018-09-10_15-16-54

  1. I think it would be useful to be able to generate a clone method simply by defining a method with return type same as the owning type, and then executing the code refactoring, without having to implement ICloneable.

Something like this: 2018-09-10_15-21-29

The name of the method need not be constrained to clone. It was just an example.

  1. Another useful and maybe not difficult to implement feature would be generating object cloning code , when you define a method with a single parameter that is the same type as the return type. Currently in this scenario "return parameter;" gets generated. I think it would be nice if instead it generated a clone method.

Something like this: 2018-09-10_15-25-15

But instead of generating "return parent;", it would generate code for cloning the object. I don't think anyone would need a refactoring that generates a method that just returns its parameter. The above suggestion is far more useful, and I think it would be what the user would expect to happen.

I hope the above feedback helps. Sorry for the late reply and cheers for the feature. :) 👍

cezarypiatek commented 6 years ago

Great feedback, thanks a lot!!! I will try to implement your suggestions.

cezarypiatek commented 6 years ago

@phoenix172 According to pt 3 How the complex properties should be mapped: rewriting property by property even if the object implement ICloneable interface

        public User Clone(User u)
        {
            return new User()
            {
                FirstName = u.FirstName,
                LastName = u.LastName,
                Age = u.Age,
                ComplexProperty = new ComplexObject{
                        Prop1 = u.ComplexProperty.Prop1,
                        Prop2 = u.ComplexProperty.Prop2
               }
            };
        }

or call Clone() method if it's accessible?

        public User Clone(User u)
        {
            return new User()
            {
                FirstName = u.FirstName,
                LastName = u.LastName,
                Age = u.Age,
                ComplexProperty = u.ComplexProperty.Clone(),
            };
        }
phoenix172 commented 6 years ago

I think utilizing the Clone method if ICloneable is implemented sounds reasonable. After all you are cloning the object and the object is offering a way to clone itself. With that said though, something like an "to explicit mapping" refactoring will be nice to have on the Clone method call.

cezarypiatek commented 6 years ago

Here's the new version that implements all your suggestions https://ci.appveyor.com/api/buildjobs/q2luj7o5s5jstwsw/artifacts/MappingGenerator%2FMappingGenerator%2FMappingGenerator.Vsix%2Fbin%2FRelease%2FMappingGenerator.vsix

Any feedback is welcome as always ;)

cezarypiatek commented 5 years ago

@phoenix172 sorry for bothering you, but I'm not sure you got the notification about last update. I would like to release this feature in few days.

phoenix172 commented 5 years ago

Yeah, sorry for the delay. I tested the feature and everything looks great.

cezarypiatek commented 5 years ago

Thanks for information and your time. I'm really appreciate.