Skip to content

Conversation

@fawdlstty
Copy link
Contributor

This submission is made to fulfill one of my requirements. I have a JSON data that is exactly the same as the current FlatBuffers definition. I need to perform serialization and deserialization (at least when adding members to the FlatBuffers structure definition, I want my deserialization code to throw an error to alert me to add the corresponding member). Observation shows that ObjectBuilder is quite close to my requirements, but it hides the member variables. Therefore, I made its member variables public.

@github-actions github-actions bot added c++ codegen Involving generating code from schema dart labels Dec 9, 2025
@fawdlstty fawdlstty marked this pull request as draft December 10, 2025 01:26
@fawdlstty fawdlstty marked this pull request as ready for review December 10, 2025 02:14
@jtdavis777
Copy link
Collaborator

@vaind mind taking a peek at this PR and leaving some thoughts?

@vaind
Copy link
Contributor

vaind commented Dec 10, 2025

@vaind mind taking a peek at this PR and leaving some thoughts?

I won't have time to do so for a while but will have a look when I do and this is still open.

@fawdlstty
Copy link
Contributor Author

@vaind I actually hope to make many incompatible modifications, such as using Object Builder as a separate object model (but without direct builder functionality), and adding serialization, deserialization, and other functions to this object model. This PR is the smallest modification made on the basis of compatibility, and I am actually not very satisfied with it.

@fawdlstty fawdlstty mentioned this pull request Dec 18, 2025
@jtdavis777
Copy link
Collaborator

@fawdlstty given

This PR is the smallest modification made on the basis of compatibility, and I am actually not very satisfied with it.

I'm not super excited about reviewing this in it's current state if this is true. I know very little/nothing about Dart, so I would have to lean on others to know if this is a feature set that is beneficial to all of the flatbuffers dart community or not.

Copy link
Contributor

@vaind vaind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a look. This is exposing internal (private) members, breaking encapsulation. As such, I would not recommend merging (I can't approve/request-request changes here directly because I'm not a maintainer).

@fawdlstty
Copy link
Contributor Author

fawdlstty commented Dec 18, 2025

The effect I need to achieve is to deserialize the JSON object generated by the Rust language, which has a one-to-one correspondence with the FBS object model, into the FBS object model. Then, read some of the variable values within it. Now, the object model generated by flatc has numerous issues that prevent it from meeting my requirements.

@vaint Additionally, I would like to clarify that I did not violate encapsulation. I merely made the current code more user-friendly. (If you disagree, you can provide your feedback and explain how the above code usage could lead to bugs.)

@jtdavis777 What I mean is not that the aforementioned pr is worthless. I hope to gradually make small adjustments within a reasonable range, so that the outcome becomes increasingly satisfactory (rather than making a huge change all at once).

@fawdlstty
Copy link
Contributor Author

@vaind It would be even better if you could offer some suggestions for modifications (such as what should be done to meet my requirements).

@vaind
Copy link
Contributor

vaind commented Dec 18, 2025

The effect I need to achieve is to deserialize the JSON object generated by the Rust language

I haven't used flatbuffers for a while so maybe this has changed but AFAIR runtime JSON de/serialization is a non-objective of this library. You can convert JSON to flatbuffers via a command-line utility if you have preexisting JSON you'd like to use. Then you can use flatbuffer in any language interchangeably.

Having a serializer and deserializer at runtime defeats the whole purpose of Flatbuffers https://flatbuffers.dev/ - e.g. "Access to serialized data without parsing/unpacking". I understand this makes sense in your usecase, it just might not be a good fit to upstream these changes to the generally available library. Which is perfectly normal in OS.

@fawdlstty
Copy link
Contributor Author

Exposing these private members break encapsulation in a wider sense that the objects implementation is now public - these properties are implementation details and can change at any point in the future without a breaking change to user code.

@vaind Thank you for your reply. If I change it to a pure object model (instead of inheriting from fb.ObjectBuilder) and make the members accessible to users, will that alleviate your concerns?

@fawdlstty
Copy link
Contributor Author

AFAIR runtime JSON de/serialization is a non-objective of this library

Yes, those are not the goals of this library. So for developers like me who have special requirements, using it is quite uncomfortable. In addition, FlatBuffers is not completely irrelevant to my requirements, but it has fulfilled half of them. Some parts I can use directly, while others require continuous improvement.

I don't think I'm the only one who has encountered this requirement (because FlatBuffers already has a large amount of code that fulfills my needs). I hope to improve it.

@fawdlstty
Copy link
Contributor Author

@jtdavis777 I don't want to spend most of my energy contributing code arguing with people who are ideologically opposed (they can easily say 'no' to features they don't need without considering the consequences). I hope to have accurate information. Have flatbuffers considered optimizing the use of this part?

@jtdavis777
Copy link
Collaborator

I have not sat down to thoroughly review this PR or the discourse yet -- it is on my list but I'm a volunteer contributing in my spare time. I will take a look when I can!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ codegen Involving generating code from schema dart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants