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

mergeFrom doesn't merge fields from other object #22

Closed
mwildehahn opened this issue Jan 12, 2015 · 6 comments
Closed

mergeFrom doesn't merge fields from other object #22

mwildehahn opened this issue Jan 12, 2015 · 6 comments
Labels

Comments

@mwildehahn
Copy link

https://github.com/alexeyxo/protobuf-swift/blob/master/src/compiler/swift_message.cc#L774-L776

given the function definition, i would expect this function to copy the fields for the "other" protobuf that is passed in.

why does it check if its the same class and then return itself? it should be the same class and then copy the attributes.

@alexeyxo
Copy link
Owner

If other is the default instance, we know none of its fields are set so we can skip the merge. This is small optimization.

@mwildehahn
Copy link
Author

image

you can see above that its not merging the contents from the other protobuf because a populated protobuf is always equal to the default instance.

i removed the return statement in mergeFrom and it works properly.

@alexeyxo
Copy link
Owner

You are trying to merge two of the same objects other, this is not true.
In this case, the other object hasn't established fields.
You can merge other and user, because user has established field.
You can look implementation in C ++ / Java.

@mwildehahn
Copy link
Author

sorry, i had a typo in there. this shows that other and user don't merge properly:

image

@mwildehahn
Copy link
Author

because user == UserService.Containers.user() they don't merge.

@alexeyxo
Copy link
Owner

message Test
{
  optional string str = 11;
}
        var builder = Test.builder()
        builder.str = "123"
        println(builder)
        var build = builder.build()
        println(build)
        var builder2 = Test.builder()
        builder2.mergeFrom(build)
        var build2 = builder2.build()

        println(build2)
        println(build2 == build)
        println(build2 == Test.builder().build())
        println(build == Test.builder().build())
        println(build == Test())

Log:

 str: 123 

 str: 123 

 str: 123 

 str: 123 

true
false
false
false

All ok!!

alexeyxo added a commit that referenced this issue Jan 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants