The "fetch" method uses wrong indexing for repeated messages/groups
Closed this issue · 2 comments
I have tested the below example with RProtobuf 0.4.9 on R version 3.4.1 (x86_64-pc-linux-gnu).
The fetch method disregards the provided index for repeated message types:
> p <- new(tutorial.Person, name = "John", id = 1)
> p$add("phone", new(tutorial.Person.PhoneNumber, number = "one"))
> p$add("phone", new(tutorial.Person.PhoneNumber, number = "two"))
> p$add("phone", new(tutorial.Person.PhoneNumber, number = "three"))
> for (i in 1:3) { cat(p$fetch("phone", i)[[1]]$toString()) }
number: "one"
number: "one"
number: "one" # expected to see: one, two, three
I believe the issue is with the get_field_values
code in wrapper_Message.cpp. The code uses raw loop variable i
instead of index[i]
for repeated messages and groups:
res[i] = S4_Message(CLONE(&ref->GetRepeatedMessage(*message, field_desc, i)));
Hence the fix should be trivial:
res[i] = S4_Message(CLONE(&ref->GetRepeatedMessage(*message, field_desc, index[i])));
Please also note that a reasonable workaround for this issue is to iterate through the repeated messages without using the fetch method:
> p <- new(tutorial.Person, name = "John", id = 1)
> p$add("phone", new(tutorial.Person.PhoneNumber, number = "one"))
> p$add("phone", new(tutorial.Person.PhoneNumber, number = "two"))
> p$add("phone", new(tutorial.Person.PhoneNumber, number = "three"))
> for (telephone in p$phone) { cat(telephone$toString()) }
number: "one"
number: "two"
number: "three"
Good analysis, this looks "likely" also based on the lines above it all using index[i]
.
Thanks again for the report, and suggested fix. The new version is now on CRAN.
Next time we can do a proper pull request so that you get proper credit (but we'd still prefer an identifiable name and email with that....)