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

Investigate how to best get rid of slice pointers #21

Open
nscuro opened this issue Feb 8, 2022 · 5 comments
Open

Investigate how to best get rid of slice pointers #21

nscuro opened this issue Feb 8, 2022 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@nscuro
Copy link
Member

nscuro commented Feb 8, 2022

Currently, we're using slice pointers (e.g. *[]string) to work around a limitation of encoding/xml.

It works, but it's not pretty. This is painfully obvious when trying to iterate over elements of those slices and having to dereference and nil check all the time. It'd be great if we could just have normal slices instead.

Most likely, we'll have to introduce a lot more custom (de-)serialization code specific to the XML format to make this work.

@nscuro nscuro added this to the v1.0.0 milestone Feb 8, 2022
@nscuro nscuro added the enhancement New feature or request label Feb 8, 2022
@nscuro nscuro added the help wanted Extra attention is needed label Jul 27, 2023
@ragaskar
Copy link

Is it known if anyone is actively working on this?

If not, and I have some spare time (in short supply for me at the moment, but you never know...) I might take a shot at fixing this, since as a user (thanks for this lib, by the way!!!!) I definitely feel the rough edges caused by these pointers. Just don't want to duplicate efforts if someone else is already working on this problem 😁 .

I imagine any extra existing context would be helpful too (e.g., "I tried doing it but it was harder than I thought because X, Y or Z").

@nscuro
Copy link
Member Author

nscuro commented Sep 25, 2024

I don't have much extra context here. The main issue is that simply switching to non-pointer slices makes the XML serialization extremely noisy, and potentially invalid according to the spec. This issue is detailed in the StackOverflow discussion I linked in the issue description.

The only alternative I can come up with is to implement MarshalXML for every single type in the code base. Which is a lot of code, and will drastically increase maintenance effort going forward.

@ragaskar
Copy link

Thanks, that's a helpful starting point. Looking through that I guess my rough understanding is that XML un-marshalling will only "omitempty" slices if they are pointers (and thus actually nil), and there's a desire to avoid emitting noisy XML -- XML with a bunch of empty tags (makes sense). I guess I would hope for a magic bullet of shared behavior (shared unmarshall code via struct composition?) that could define this in one place. Anyways, if I get a chance to tinker I'll try to share anything I learn on this issue.

@ragaskar
Copy link

ragaskar commented Oct 2, 2024

Thought about this a little bit, and one thing stopping me from testing out some implementations: how would the library approach the breaking change this introduces?

When I look at the code I've written that would "benefit" from this, it would all need to be rewritten, so as a user I would want a long deprecation period.

Would one opt-in at instantiation time (with a flag/diff struct) to get the pointer-free structure? It seems like it would be potentially a massive pain to maintain parallel structures internally.

@nscuro
Copy link
Member Author

nscuro commented Oct 2, 2024

how would the library approach the breaking change this introduces?

We abide to SemVer and release that change under a new major version. We can maintain multiple major versions if required, but I don't see a need to maintain two separate implementations within the same version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants