Skip to content

Comments

Implement OneOfStructJsonConverter that serializes OneOf structures with value and reference arguments.#4

Open
r-arjocu wants to merge 1 commit intodpraimeyuu:masterfrom
r-arjocu:OOS-2
Open

Implement OneOfStructJsonConverter that serializes OneOf structures with value and reference arguments.#4
r-arjocu wants to merge 1 commit intodpraimeyuu:masterfrom
r-arjocu:OOS-2

Conversation

@r-arjocu
Copy link

@r-arjocu r-arjocu commented Sep 13, 2023

OneOfStructJsonConverter sources with T0-T2 generic arguments and tests for them are provided. Unfortunately I don't know how to write code that generates code (and don't have the time to learn now), but additional implementations with T3 through T8 generic arguments can be easily implemented by copy-paste with minor adjustments and appropriate tests.
The implementation doesn't cover all value types according to Newtonsoft's JTokenType, but these can be relatively easily added with the appropriate tests.

The reasoning behind a second JSON converter in the repo is that I could not make OneOfJsonConverter work correctly directly with OneOf<> structure, because of the latter's restrictions enforced by its design -- I'm certain you're quite aware of them. So I needed a different name for the converter, because of the variant with only T0 generic argument that clashes with OneOfJsonConverter. Do you think that OneOfStructJsonConverter is a good name?

…nd tests, that cover the serialization of OneOf structures with value and reference arguments.
@dpraimeyuu
Copy link
Owner

@ra-mt Sorry for a long time without any answer from my side.

I am totally fine with having a second converter, as long as it gets properly documented by telling the differences and mainly why it exists.

I don't have any "roadmap" so if it helps to solve your problem - I am totally for adding it.

@dpraimeyuu
Copy link
Owner

@ra-mt
What would you say about "namespacing" it by putting classes added by you in a static class:
image?

Alternatively, adding a proper namespace:

  • OneOf.Serialization.Converters.OneOfJsonConverter
  • OneOf.Serialization.Converters.MixedRefsAndStructs.OneOfJsonConverter (it's long but descriptive)

@r-arjocu
Copy link
Author

r-arjocu commented Jan 19, 2024

Unfortunately, I'm changed jobs right now and have other bigger hurdles. I don't expect to be able to contribute for some time; as I see it, I may forget about this. So, my idea is following: you merge what I have provided in this pull request, and from my point of view you can change the code layout (introduce namespaces, static classes etc.) as you deem feet (I won't mind). What do yo say?

I would name it MixedRefsAndValues instead of MixedRefsAndStructs, though, to better describe the intention -- "Refs" and "Values" refer to the OneOf members, whicle "Struct" from OneOfStructJsonConverter refers to OneOf as a struct. Thanks!

@r-arjocu
Copy link
Author

r-arjocu commented Jan 23, 2024

Hi. I will change my username to r-arjocu. I'm unsure how this will effect the pull req.

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