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

[API Proposal]: Bulk copy properties from one object to another #101676

Closed
NETKroks opened this issue Apr 29, 2024 · 11 comments
Closed

[API Proposal]: Bulk copy properties from one object to another #101676

NETKroks opened this issue Apr 29, 2024 · 11 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Reflection

Comments

@NETKroks
Copy link

NETKroks commented Apr 29, 2024

Background and motivation

Problem
The

obj1.PropertyOne = obj2.PropertyOne;
obj1.PropertyTwo = obj2.PropertyTwo;
...

pattern is very inconvenient to write out if the purpose is just to Update all properties of one object to the properties of another one (same type).

Also this approach is not the fastest possible if we consider raw byte bulk copying fields from one object to another (assuming same type)

Where does this pattern occur?
One potential Use-Case: Updating an Entity by using Entity Framework. I do not want to type this pattern out for every property:

Another one would be: you keep track of objects in-memory and you provide an Update API. The User can pass an object with the same type to the method and you want to update all properties.

More explicit example using ASP:

   [HttpPatch("...")]
    public async Task<IActionResult> UpdateCustomType([FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Disallow)] CustomType obj2)
    {
        var obj1 = CustomTypeManager.GetById(obj2.Id);
        
        obj1.PropertyOne = obj2.PropertyOne;
        obj1.PropertyTwo = obj2.PropertyTwo ;
        ...
        
        return Ok();
    }

In this example I may not be able just to replace the entire object since the object could be referenced multiple times by other things. The old reference would not be available anymore.

API Proposal

We could instead introduce Unsafe.CopyProperties<T>(from: t1, to: t2) which would basically bulk copy all field values from obj1 to obj2 (thus the properties are changed since they just get and set the backing fields). Since obj1 and obj2 are equal in type, we can even assume its safe to copy raw bytes over to the other object.
This would make it a lot more convenient as you do not need to type out every property. And it would be faster than the reflection approach.

API Usage

It would look like this:

   [HttpPatch("...")]
    public async Task<IActionResult> UpdateCustomType([FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Disallow)] CustomType obj2)
    {
        var obj1 = CustomTypeManager.GetById(obj2.Id);
        Unsafe.CopyProperties(from: obj2, to: obj1);
        return Ok();
    }

Alternative Designs

No response

Risks

Raw byte bulk-copy may be possible or not, that depends on the internals which needs to be discussed.

@NETKroks NETKroks added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 29, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 29, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 29, 2024
@jkotas jkotas added area-System.Reflection and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 29, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member

jkotas commented Apr 29, 2024

Unsafe.CopyProperties(from: t1, to: t2)

This is a proposal for a reflection heavy API. It is not appropriate to be on Unsafe.* that is reserved for low-level unsafe APIs.

Alternative Designs

If you do not want to type this pattern out for every property, it sounds like a job for a source generator.

@EgorBo
Copy link
Member

EgorBo commented Apr 29, 2024

I assume a de-facto standard tool for this is https://github.com/AutoMapper/AutoMapper, I guess there are SG-based options too

@NETKroks
Copy link
Author

Unsafe.CopyProperties(from: t1, to: t2)

This is a proposal for a reflection heavy API. It is not appropriate to be on Unsafe.* that is reserved for low-level unsafe APIs.

Alternative Designs

If you do not want to type this pattern out for every property, it sounds like a job for a source generator.

It Operates on a lower level as it Bulk copies field values on raw Level. Since the types are equal (of both objects) we can operate on a lower Level. Also the bulk copying might be faster than writing the assignment Statements by Hand

@CyrusNajmabadi
Copy link
Member

pattern is very inconvenient to write out if the purpose is just to Update all properties of one object to the properties of another one (same type).

But the pattern is solved already with source generators.

It Operates on a lower level

Why is that important here. The motivation was listed as the inconvenience of manually writing it. There's no motivation provided for needing something "lower level".

@NETKroks
Copy link
Author

pattern is very inconvenient to write out if the purpose is just to Update all properties of one object to the properties of another one (same type).

But the pattern is solved already with source generators.

It Operates on a lower level

Why is that important here. The motivation was listed as the inconvenience of manually writing it. There's no motivation provided for needing something "lower level".

This was in reference to the complaint about the method being in Unsafe class.

@huoyaoyuan
Copy link
Member

This is a proposal for a reflection heavy API.

Instead, is there an API to copy all fields to another object? Like MemberwiseClone but uses an existing object identity.

@NETKroks
Copy link
Author

NETKroks commented Apr 29, 2024

This is a proposal for a reflection heavy API.

Instead, is there an API to copy all fields to another object? Like MemberwiseClone but uses an existing object identity.

There currently seems to not be any approach. MemberwiseClone, as you already said, creates a new instance. My approach is a Copy From->To approach which uses existing references.

@colejohnson66
Copy link

Are you suggesting a bitwise copy?

@CyrusNajmabadi
Copy link
Member

My approach is a Copy From->To approach which uses existing references.

And my source generator question still applies here.

@steveharter
Copy link
Member

steveharter commented May 1, 2024

Closing; several edge cases and concerns:

  • The "CopyProperties" would copy all data with no way to limit.
  • Would have very niche scenarios (use EF was mentioned)
  • Readonly fields may be an issue \ concern
  • Is not AOT-friendly; as mentioned above perhaps a source generator approach would be better.
  • What if types don't match exactly due to inheritance?
  • Copies fields, not properties, so is misnamed.
  • Is easy to implement a helper method or extension method that handles the scenario.

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label May 1, 2024
@buyaa-n buyaa-n closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Reflection
Projects
None yet
Development

No branches or pull requests

8 participants