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

Pass by value in MarshalXML. #55

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Pass by value in MarshalXML. #55

merged 1 commit into from
Apr 10, 2024

Conversation

dtbartle
Copy link
Contributor

@dtbartle dtbartle commented Apr 7, 2024

This fixes XML marshaling when these types are embedded in structs as values (vs pointers). The specific use case I have is to store the raw XML values for later processing (and unmarshaling).

I don't think this fix is complete. For example, the InvTranList type has the same problem, though in practice it is always embedded in a struct by pointer. I think the XML parser could check for both the marshaler interface on the type and the marshier interface on the pointer to the type when iterating through structs.

@aclindsa
Copy link
Owner

In general, this looks reasonable to me, thanks for the contribution!

The specific use case I have is to store the raw XML values for later processing (and unmarshaling).

Just out of curiosity, why do you want/need to do this? I don't think I'd conceived of a need to do so.

I don't think this fix is complete. For example, the InvTranList type has the same problem, though in practice it is always embedded in a struct by pointer.

Are you saying you're planning to this in a future MR, or just suggesting it could hypothetically be done?

@aclindsa aclindsa merged commit 4f2c558 into aclindsa:master Apr 10, 2024
20 checks passed
@dtbartle
Copy link
Contributor Author

I have a personal finances project that tracks balances/transactions. I currently only use some of the information from the responses, but it felt nicer to save the XML in case I ever decide to pull in additional fields in the future.

I wasn't going to make subsequent fixes; changing some large structs to be value receivers would, I think, mean we'd be copying larger structs a bunch for marshaling (though maybe this is fine, as probably nobody is using OFX for high-performance stuff).

@aclindsa
Copy link
Owner

Neat. And sounds good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants